test rebase

ci/matrix.sh

@@ -40,7 +40,7 @@ if [ "$BUILD_TARGET" = "arm-linux" ]; then
   export CHECK_DOC=1
   # -Wno-psabi is to disable ABI warnings: "note: parameter passing for argument of type ... changed in GCC 7.1"
   # This could be removed once the ABI change warning does not show up by default
-  export BITCOIN_CONFIG="--enable-glibc-back-compat --enable-reduce-exports CXXFLAGS=-Wno-psabi"
+  export BITCOIN_CONFIG="--enable-glibc-back-compat --enable-reduce-exports --enable-werror CXXFLAGS=-Wno-psabi"
   export RUN_UNITTESTS=false
   export RUN_INTEGRATIONTESTS=false
 elif [ "$BUILD_TARGET" = "win32" ]; then
@@ -67,7 +67,7 @@ elif [ "$BUILD_TARGET" = "linux64" ]; then
 elif [ "$BUILD_TARGET" = "linux64_cxx17" ]; then
   export HOST=x86_64-unknown-linux-gnu
   export DEP_OPTS="NO_UPNP=1 DEBUG=1"
-  export BITCOIN_CONFIG="--enable-zmq --enable-glibc-back-compat --enable-reduce-exports --enable-crash-hooks --enable-c++17 --with-sanitizers=undefined"
+  export BITCOIN_CONFIG="--enable-zmq --enable-glibc-back-compat --enable-reduce-exports --enable-crash-hooks --enable-c++17 --enable-werror --with-sanitizers=undefined"
   export CPPFLAGS="-DDEBUG_LOCKORDER -DENABLE_DASH_DEBUG -DARENA_DEBUG"
   export PYZMQ=true
   export RUN_INTEGRATIONTESTS=false

src/bench/bench_dash.cpp

@@ -20,7 +20,6 @@
 
 const std::function<std::string(const char*)> G_TRANSLATION_FUN = nullptr;
 
-static const int64_t DEFAULT_BENCH_EVALUATIONS = 5;
 static const char* DEFAULT_BENCH_FILTER = ".*";
 
 void InitBLSTests();

src/bench/bls_dkg.cpp

@@ -86,8 +86,6 @@ class DKG
         ReceiveVvecs();
         size_t memberIdx = 0;
         bench.minEpochIterations(epoch_iters).run([&] {
-            auto& m = members[memberIdx];
-
             ReceiveShares(memberIdx);
 
             std::set<size_t> invalidIndexes;

src/bench/checkqueue.cpp

@@ -11,7 +11,6 @@
 #include <random.h>
 
 
-static const int MIN_CORES = 2;
 static const size_t BATCHES = 101;
 static const size_t BATCH_SIZE = 30;
 static const int PREVECTOR_SIZE = 28;

src/coinjoin/coinjoin-server.cpp

@@ -56,7 +56,7 @@ void CCoinJoinServer::ProcessMessage(CNode* pfrom, const std::string& strCommand
         LogPrint(BCLog::COINJOIN, "DSACCEPT -- nDenom %d (%s)  txCollateral %s", dsa.nDenom, CCoinJoin::DenominationToString(dsa.nDenom), dsa.txCollateral.ToString()); /* Continued */
 
         auto mnList = deterministicMNManager->GetListAtChainTip();
-        auto dmn = mnList.GetValidMNByCollateral(activeMasternodeInfo.outpoint);
+        auto dmn = WITH_LOCK(activeMasternodeInfoCs, return mnList.GetValidMNByCollateral(activeMasternodeInfo.outpoint));
         if (!dmn) {
             PushStatus(pfrom, STATUS_REJECTED, ERR_MN_LIST, connman);
             return;
@@ -68,7 +68,7 @@ void CCoinJoinServer::ProcessMessage(CNode* pfrom, const std::string& strCommand
                 if (!lockRecv) return;
 
                 for (const auto& q : vecCoinJoinQueue) {
-                    if (q.masternodeOutpoint == activeMasternodeInfo.outpoint) {
+                    if (WITH_LOCK(activeMasternodeInfoCs, return q.masternodeOutpoint == activeMasternodeInfo.outpoint)) {
                         // refuse to create another queue this often
                         LogPrint(BCLog::COINJOIN, "DSACCEPT -- last dsq is still in queue, refuse to mix\n");
                         PushStatus(pfrom, STATUS_REJECTED, ERR_RECENT, connman);
@@ -334,7 +334,7 @@ void CCoinJoinServer::CommitFinalTransaction(CConnman& connman)
 
     // create and sign masternode dstx transaction
     if (!CCoinJoin::GetDSTX(hashTx)) {
-        CCoinJoinBroadcastTx dstxNew(finalTransaction, activeMasternodeInfo.outpoint, GetAdjustedTime());
+        CCoinJoinBroadcastTx dstxNew(finalTransaction, WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.outpoint), GetAdjustedTime());
         dstxNew.Sign();
         CCoinJoin::AddDSTX(dstxNew);
     }
@@ -501,7 +501,7 @@ void CCoinJoinServer::CheckForCompleteQueue(CConnman& connman)
     if (nState == POOL_STATE_QUEUE && IsSessionReady()) {
         SetState(POOL_STATE_ACCEPTING_ENTRIES);
 
-        CCoinJoinQueue dsq(nSessionDenom, activeMasternodeInfo.outpoint, GetAdjustedTime(), true);
+        CCoinJoinQueue dsq(nSessionDenom, WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.outpoint), GetAdjustedTime(), true);
         LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CheckForCompleteQueue -- queue is ready, signing and relaying (%s) " /* Continued */
                                      "with %d participants\n", dsq.ToString(), vecSessionCollaterals.size());
         dsq.Sign();
@@ -708,7 +708,7 @@ bool CCoinJoinServer::CreateNewSession(const CCoinJoinAccept& dsa, PoolMessage&
 
     if (!fUnitTest) {
         //broadcast that I'm accepting entries, only if it's the first entry through
-        CCoinJoinQueue dsq(nSessionDenom, activeMasternodeInfo.outpoint, GetAdjustedTime(), false);
+        CCoinJoinQueue dsq(nSessionDenom, WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.outpoint), GetAdjustedTime(), false);
         LogPrint(BCLog::COINJOIN, "CCoinJoinServer::CreateNewSession -- signing and relaying new queue: %s\n", dsq.ToString());
         dsq.Sign();
         dsq.Relay(connman);

src/coinjoin/coinjoin.cpp

@@ -50,7 +50,7 @@ bool CCoinJoinQueue::Sign()
 
 
     uint256 hash = GetSignatureHash();
-    CBLSSignature sig = activeMasternodeInfo.blsKeyOperator->Sign(hash);
+    CBLSSignature sig = WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.blsKeyOperator->Sign(hash));
     if (!sig.IsValid()) {
         return false;
     }
@@ -96,7 +96,7 @@ bool CCoinJoinBroadcastTx::Sign()
 
     uint256 hash = GetSignatureHash();
 
-    CBLSSignature sig = activeMasternodeInfo.blsKeyOperator->Sign(hash);
+    CBLSSignature sig = WITH_LOCK(activeMasternodeInfoCs, return activeMasternodeInfo.blsKeyOperator->Sign(hash));
     if (!sig.IsValid()) {
         return false;
     }

src/evo/mnauth.cpp

@@ -18,31 +18,30 @@
 
 void CMNAuth::PushMNAUTH(CNode* pnode, CConnman& connman)
 {
+    LOCK(activeMasternodeInfoCs);
     if (!fMasternodeMode || activeMasternodeInfo.proTxHash.IsNull()) {
         return;
     }
 
     uint256 signHash;
-    {
-        LOCK(pnode->cs_mnauth);
-        if (pnode->receivedMNAuthChallenge.IsNull()) {
-            return;
-        }
-        // We include fInbound in signHash to forbid interchanging of challenges by a man in the middle (MITM). This way
-        // we protect ourselves against MITM in this form:
-        //   node1 <- Eve -> node2
-        // It does not protect against:
-        //   node1 -> Eve -> node2
-        // This is ok as we only use MNAUTH as a DoS protection and not for sensitive stuff
-        int nOurNodeVersion{PROTOCOL_VERSION};
-        if (Params().NetworkIDString() != CBaseChainParams::MAIN && gArgs.IsArgSet("-pushversion")) {
-            nOurNodeVersion = gArgs.GetArg("-pushversion", PROTOCOL_VERSION);
-        }
-        if (pnode->nVersion < MNAUTH_NODE_VER_VERSION || nOurNodeVersion < MNAUTH_NODE_VER_VERSION) {
-            signHash = ::SerializeHash(std::make_tuple(*activeMasternodeInfo.blsPubKeyOperator, pnode->receivedMNAuthChallenge, pnode->fInbound));
-        } else {
-            signHash = ::SerializeHash(std::make_tuple(*activeMasternodeInfo.blsPubKeyOperator, pnode->receivedMNAuthChallenge, pnode->fInbound, nOurNodeVersion));
-        }
+    auto receivedMNAuthChallenge = pnode->GetReceivedMNAuthChallenge();
+    if (receivedMNAuthChallenge.IsNull()) {
+        return;
+    }
+    // We include fInbound in signHash to forbid interchanging of challenges by a man in the middle (MITM). This way
+    // we protect ourselves against MITM in this form:
+    //   node1 <- Eve -> node2
+    // It does not protect against:
+    //   node1 -> Eve -> node2
+    // This is ok as we only use MNAUTH as a DoS protection and not for sensitive stuff
+    int nOurNodeVersion{PROTOCOL_VERSION};
+    if (Params().NetworkIDString() != CBaseChainParams::MAIN && gArgs.IsArgSet("-pushversion")) {
+        nOurNodeVersion = gArgs.GetArg("-pushversion", PROTOCOL_VERSION);
+    }
+    if (pnode->nVersion < MNAUTH_NODE_VER_VERSION || nOurNodeVersion < MNAUTH_NODE_VER_VERSION) {
+        signHash = ::SerializeHash(std::make_tuple(*activeMasternodeInfo.blsPubKeyOperator, receivedMNAuthChallenge, pnode->fInbound));
+    } else {
+        signHash = ::SerializeHash(std::make_tuple(*activeMasternodeInfo.blsPubKeyOperator, receivedMNAuthChallenge, pnode->fInbound, nOurNodeVersion));
     }
 
     CMNAuth mnauth;
@@ -66,11 +65,7 @@ void CMNAuth::ProcessMessage(CNode* pnode, const std::string& strCommand, CDataS
         vRecv >> mnauth;
 
         // only one MNAUTH allowed
-        bool fAlreadyHaveMNAUTH = false;
-        {
-            LOCK(pnode->cs_mnauth);
-            fAlreadyHaveMNAUTH = !pnode->verifiedProRegTxHash.IsNull();
-        }
+        bool fAlreadyHaveMNAUTH = !pnode->GetVerifiedProRegTxHash().IsNull();
         if (fAlreadyHaveMNAUTH) {
             LOCK(cs_main);
             Misbehaving(pnode->GetId(), 100, "duplicate mnauth");
@@ -108,20 +103,17 @@ void CMNAuth::ProcessMessage(CNode* pnode, const std::string& strCommand, CDataS
         }
 
         uint256 signHash;
-        {
-            LOCK(pnode->cs_mnauth);
-            int nOurNodeVersion{PROTOCOL_VERSION};
-            if (Params().NetworkIDString() != CBaseChainParams::MAIN && gArgs.IsArgSet("-pushversion")) {
-                nOurNodeVersion = gArgs.GetArg("-pushversion", PROTOCOL_VERSION);
-            }
-            // See comment in PushMNAUTH (fInbound is negated here as we're on the other side of the connection)
-            if (pnode->nVersion < MNAUTH_NODE_VER_VERSION || nOurNodeVersion < MNAUTH_NODE_VER_VERSION) {
-                signHash = ::SerializeHash(std::make_tuple(dmn->pdmnState->pubKeyOperator, pnode->sentMNAuthChallenge, !pnode->fInbound));
-            } else {
-                signHash = ::SerializeHash(std::make_tuple(dmn->pdmnState->pubKeyOperator, pnode->sentMNAuthChallenge, !pnode->fInbound, pnode->nVersion.load()));
-            }
-            LogPrint(BCLog::NET_NETCONN, "CMNAuth::%s -- constructed signHash for nVersion %d, peer=%d\n", __func__, pnode->nVersion, pnode->GetId());
+        int nOurNodeVersion{PROTOCOL_VERSION};
+        if (Params().NetworkIDString() != CBaseChainParams::MAIN && gArgs.IsArgSet("-pushversion")) {
+            nOurNodeVersion = gArgs.GetArg("-pushversion", PROTOCOL_VERSION);
         }
+        // See comment in PushMNAUTH (fInbound is negated here as we're on the other side of the connection)
+        if (pnode->nVersion < MNAUTH_NODE_VER_VERSION || nOurNodeVersion < MNAUTH_NODE_VER_VERSION) {
+            signHash = ::SerializeHash(std::make_tuple(dmn->pdmnState->pubKeyOperator, pnode->GetSentMNAuthChallenge(), !pnode->fInbound));
+        } else {
+            signHash = ::SerializeHash(std::make_tuple(dmn->pdmnState->pubKeyOperator, pnode->GetSentMNAuthChallenge(), !pnode->fInbound, pnode->nVersion.load()));
+        }
+        LogPrint(BCLog::NET_NETCONN, "CMNAuth::%s -- constructed signHash for nVersion %d, peer=%d\n", __func__, pnode->nVersion, pnode->GetId());
 
         if (!mnauth.sig.VerifyInsecure(dmn->pdmnState->pubKeyOperator.Get(), signHash)) {
             LOCK(cs_main);
@@ -147,12 +139,12 @@ void CMNAuth::ProcessMessage(CNode* pnode, const std::string& strCommand, CDataS
                 return;
             }
 
-            if (pnode2->verifiedProRegTxHash == mnauth.proRegTxHash) {
+            if (pnode2->GetVerifiedProRegTxHash() == mnauth.proRegTxHash) {
                 if (fMasternodeMode) {
-                    auto deterministicOutbound = llmq::CLLMQUtils::DeterministicOutboundConnection(activeMasternodeInfo.proTxHash, mnauth.proRegTxHash);
+                    auto deterministicOutbound = WITH_LOCK(activeMasternodeInfoCs, return llmq::CLLMQUtils::DeterministicOutboundConnection(activeMasternodeInfo.proTxHash, mnauth.proRegTxHash));
                     LogPrint(BCLog::NET_NETCONN, "CMNAuth::ProcessMessage -- Masternode %s has already verified as peer %d, deterministicOutbound=%s. peer=%d\n",
                              mnauth.proRegTxHash.ToString(), pnode2->GetId(), deterministicOutbound.ToString(), pnode->GetId());
-                    if (deterministicOutbound == activeMasternodeInfo.proTxHash) {
+                    if (WITH_LOCK(activeMasternodeInfoCs, return deterministicOutbound == activeMasternodeInfo.proTxHash)) {
                         if (pnode2->fInbound) {
                             LogPrint(BCLog::NET_NETCONN, "CMNAuth::ProcessMessage -- dropping old inbound, peer=%d\n", pnode2->GetId());
                             pnode2->fDisconnect = true;
@@ -181,13 +173,10 @@ void CMNAuth::ProcessMessage(CNode* pnode, const std::string& strCommand, CDataS
             return;
         }
 
-        {
-            LOCK(pnode->cs_mnauth);
-            pnode->verifiedProRegTxHash = mnauth.proRegTxHash;
-            pnode->verifiedPubKeyHash = dmn->pdmnState->pubKeyOperator.GetHash();
-        }
+        pnode->SetVerifiedProRegTxHash(mnauth.proRegTxHash);
+        pnode->SetVerifiedPubKeyHash(dmn->pdmnState->pubKeyOperator.GetHash());
 
-        if (!pnode->m_masternode_iqr_connection && connman.IsMasternodeQuorumRelayMember(pnode->verifiedProRegTxHash)) {
+        if (!pnode->m_masternode_iqr_connection && connman.IsMasternodeQuorumRelayMember(pnode->GetVerifiedProRegTxHash())) {
             // Tell our peer that we're interested in plain LLMQ recovered signatures.
             // Otherwise the peer would only announce/send messages resulting from QRECSIG,
             // e.g. InstantSend locks or ChainLocks. SPV and regular full nodes should not send
@@ -209,11 +198,11 @@ void CMNAuth::NotifyMasternodeListChanged(bool undo, const CDeterministicMNList&
     }
 
     g_connman->ForEachNode([&](CNode* pnode) {
-        LOCK(pnode->cs_mnauth);
-        if (pnode->verifiedProRegTxHash.IsNull()) {
+        auto verifiedProRegTxHash = pnode->GetVerifiedProRegTxHash();
+        if (verifiedProRegTxHash.IsNull()) {
             return;
         }
-        auto verifiedDmn = oldMNList.GetMN(pnode->verifiedProRegTxHash);
+        auto verifiedDmn = oldMNList.GetMN(verifiedProRegTxHash);
         if (!verifiedDmn) {
             return;
         }
@@ -223,15 +212,15 @@ void CMNAuth::NotifyMasternodeListChanged(bool undo, const CDeterministicMNList&
         } else {
             auto it = diff.updatedMNs.find(verifiedDmn->GetInternalId());
             if (it != diff.updatedMNs.end()) {
-                if ((it->second.fields & CDeterministicMNStateDiff::Field_pubKeyOperator) && it->second.state.pubKeyOperator.GetHash() != pnode->verifiedPubKeyHash) {
+                if ((it->second.fields & CDeterministicMNStateDiff::Field_pubKeyOperator) && it->second.state.pubKeyOperator.GetHash() != pnode->GetVerifiedPubKeyHash()) {
                     doRemove = true;
                 }
             }
         }
 
         if (doRemove) {
             LogPrint(BCLog::NET_NETCONN, "CMNAuth::NotifyMasternodeListChanged -- Disconnecting MN %s due to key changed/removed, peer=%d\n",
-                     pnode->verifiedProRegTxHash.ToString(), pnode->GetId());
+                     pnode->GetVerifiedProRegTxHash().ToString(), pnode->GetId());
             pnode->fDisconnect = true;
         }
     });

src/hdchain.h

@@ -74,6 +74,7 @@ class CHDChain
 
         // by swapping the members of two classes,
         // the two classes are effectively swapped
+        LOCK2(first.cs, second.cs);
         swap(first.nVersion, second.nVersion);
         swap(first.id, second.id);
         swap(first.fCrypted, second.fCrypted);

src/init.cpp

@@ -363,9 +363,12 @@ void PrepareShutdown()
         UnregisterValidationInterface(activeMasternodeManager);
     }
 
-    // make sure to clean up BLS keys before global destructors are called (they have allocated from the secure memory pool)
-    activeMasternodeInfo.blsKeyOperator.reset();
-    activeMasternodeInfo.blsPubKeyOperator.reset();
+    {
+        LOCK(activeMasternodeInfoCs);
+        // make sure to clean up BLS keys before global destructors are called (they have allocated from the secure memory pool)
+        activeMasternodeInfo.blsKeyOperator.reset();
+        activeMasternodeInfo.blsPubKeyOperator.reset();
+    }
 
 #ifndef WIN32
     try {
@@ -2061,7 +2064,6 @@ bool AppInitMain()
     LogPrintf("* Using %.1fMiB for in-memory UTXO set (plus up to %.1fMiB of unused mempool space)\n", nCoinCacheUsage * (1.0 / 1024 / 1024), nMempoolSizeMax * (1.0 / 1024 / 1024));
 
     bool fLoaded = false;
-    int64_t nStart = GetTimeMillis();
 
     while (!fLoaded && !ShutdownRequested()) {
         bool fReset = fReindex;
@@ -2324,8 +2326,12 @@ bool AppInitMain()
             return InitError(_("Invalid masternodeblsprivkey. Please see documentation."));
         }
         fMasternodeMode = true;
-        activeMasternodeInfo.blsKeyOperator = std::make_unique<CBLSSecretKey>(keyOperator);
-        activeMasternodeInfo.blsPubKeyOperator = std::make_unique<CBLSPublicKey>(activeMasternodeInfo.blsKeyOperator->GetPublicKey());
+        {
+            LOCK(activeMasternodeInfoCs);
+            activeMasternodeInfo.blsKeyOperator = std::make_unique<CBLSSecretKey>(keyOperator);
+            activeMasternodeInfo.blsPubKeyOperator = std::make_unique<CBLSPublicKey>(
+                    activeMasternodeInfo.blsKeyOperator->GetPublicKey());
+        }
         LogPrintf("MASTERNODE:\n");
         LogPrintf("  blsPubKeyOperator: %s\n", keyOperator.GetPublicKey().ToString());
     }
@@ -2336,11 +2342,14 @@ bool AppInitMain()
         RegisterValidationInterface(activeMasternodeManager);
     }
 
-    if (activeMasternodeInfo.blsKeyOperator == nullptr) {
-        activeMasternodeInfo.blsKeyOperator = std::make_unique<CBLSSecretKey>();
-    }
-    if (activeMasternodeInfo.blsPubKeyOperator == nullptr) {
-        activeMasternodeInfo.blsPubKeyOperator = std::make_unique<CBLSPublicKey>();
+    {
+        LOCK(activeMasternodeInfoCs);
+        if (activeMasternodeInfo.blsKeyOperator == nullptr) {
+            activeMasternodeInfo.blsKeyOperator = std::make_unique<CBLSSecretKey>();
+        }
+        if (activeMasternodeInfo.blsPubKeyOperator == nullptr) {
+            activeMasternodeInfo.blsPubKeyOperator = std::make_unique<CBLSPublicKey>();
+        }
     }
 
     // ********************************************************* Step 10b: setup CoinJoin