A pack of small fixes (#1992)

* Make sure gobject collateral was mined

`CGovernanceObject::IsCollateralValid()` would still fail for non-mined collateral later trying to check confirmations

* Fix powLimit for mainnet/testnet

That's a legacy thing, slightly rising it to match the actual bit-shifted value has no effect because real values are already lower.
Also clarify values for all networks in comments.

* Check for script addresses in CSuperblock::ParsePaymentSchedule()

Sentinel should already be downvoting such triggers if they would exist, no need to store/relay them.

* Do not process already known valid vote twice in CGovernanceObject::ProcessVote()

This should be handled by `CGovernanceManager::ProcessVote()` but imo it's better to have this at the `CGovernanceObject::ProcessVote()` level as well.

* Make sure CGovernanceObjectVoteFile::AddVote() never adds/updates already known votes

The way `CGovernanceObjectVoteFile::AddVote()` is used (i.e. wrapped in `CGovernanceObjectVoteFile::HasVote()` condition) it's already the case. Hoever nothing would guarantee consistency if it would be used elsewhere without such wrapper, so it's better to have similar check inside.

* Do not even try mnb recovery when -connect is set

`CConnman::ThreadOpenMasternodeConnections()` thread won't even start when `-connect` is set, so no need to ask for recovery, there is nothing that is going to be able to process such request.

* No need for SIGHASH_ANYONECANPAY in PS collateral signature

Collateral is just a normal tx created and signed by each participant individually, there is no need for special sig types.

* Release semMasternodeOutbound in CConnman::Interrupt()

Re-align Dash code with Bitcoin.

src/chainparams.cpp

@@ -142,7 +142,7 @@ public:
         consensus.BIP65Height = 619382; // 00000000000076d8fcea02ec0963de4abfd01e771fec0863f960c2c64fe6f357
         consensus.BIP66Height = 245817; // 00000000000b1fa2dfa312863570e13fae9ca7b5566cb27e55422620b469aefa
         consensus.DIP0001Height = 782208;
-        consensus.powLimit = uint256S("00000fffff000000000000000000000000000000000000000000000000000000");
+        consensus.powLimit = uint256S("00000fffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"); // ~uint256(0) >> 20
         consensus.nPowTargetTimespan = 24 * 60 * 60; // Dash: 1 day
         consensus.nPowTargetSpacing = 2.5 * 60; // Dash: 2.5 minutes
         consensus.fPowAllowMinDifficultyBlocks = false;
@@ -295,7 +295,7 @@ public:
         consensus.BIP65Height = 2431; // 0000039cf01242c7f921dcb4806a5994bc003b48c1973ae0c89b67809c2bb2ab
         consensus.BIP66Height = 2075; // 0000002acdd29a14583540cb72e1c5cc83783560e38fa7081495d474fe1671f7
         consensus.DIP0001Height = 5500;
-        consensus.powLimit = uint256S("00000fffff000000000000000000000000000000000000000000000000000000");
+        consensus.powLimit = uint256S("00000fffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"); // ~uint256(0) >> 20
         consensus.nPowTargetTimespan = 24 * 60 * 60; // Dash: 1 day
         consensus.nPowTargetSpacing = 2.5 * 60; // Dash: 2.5 minutes
         consensus.fPowAllowMinDifficultyBlocks = true;
@@ -425,7 +425,7 @@ public:
         consensus.BIP65Height = 2; // BIP65 activated immediately on devnet
         consensus.BIP66Height = 2; // BIP66 activated immediately on devnet
         consensus.DIP0001Height = 2; // DIP0001 activated immediately on devnet
-        consensus.powLimit = uint256S("7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff");
+        consensus.powLimit = uint256S("7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"); // ~uint256(0) >> 1
         consensus.nPowTargetTimespan = 24 * 60 * 60; // Dash: 1 day
         consensus.nPowTargetSpacing = 2.5 * 60; // Dash: 2.5 minutes
         consensus.fPowAllowMinDifficultyBlocks = true;
@@ -553,7 +553,7 @@ public:
         consensus.BIP65Height = 1351; // BIP65 activated on regtest (Used in rpc activation tests)
         consensus.BIP66Height = 1251; // BIP66 activated on regtest (Used in rpc activation tests)
         consensus.DIP0001Height = 2000;
-        consensus.powLimit = uint256S("7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff");
+        consensus.powLimit = uint256S("7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"); // ~uint256(0) >> 1
         consensus.nPowTargetTimespan = 24 * 60 * 60; // Dash: 1 day
         consensus.nPowTargetSpacing = 2.5 * 60; // Dash: 2.5 minutes
         consensus.fPowAllowMinDifficultyBlocks = true;

src/governance-classes.cpp

@@ -598,6 +598,18 @@ void CSuperblock::ParsePaymentSchedule(const std::string& strPaymentAddresses, c
             LogPrintf("%s\n", ostr.str());
             throw std::runtime_error(ostr.str());
         }
+        /*
+            TODO
+
+            - There might be an issue with multisig in the coinbase on mainnet, we will add support for it in a future release.
+            - Post 12.3+ (test multisig coinbase transaction)
+        */
+        if(address.IsScript()) {
+            std::ostringstream ostr;
+            ostr << "CSuperblock::ParsePaymentSchedule -- Script addresses are not supported yet : " <<  vecParsed1[i];
+            LogPrintf("%s\n", ostr.str());
+            throw std::runtime_error(ostr.str());
+        }
 
         DBG( std::cout << "CSuperblock::ParsePaymentSchedule i = " << i
              <<  ", vecParsed2[i] = " << vecParsed2[i]

src/governance-object.cpp

@@ -103,6 +103,16 @@ bool CGovernanceObject::ProcessVote(CNode* pfrom,
 {
     LOCK(cs);
 
+    // do not process already known valid votes twice
+    if (fileVotes.HasVote(vote.GetHash())) {
+        // nothing to do here, not an error
+        std::ostringstream ostr;
+        ostr << "CGovernanceObject::ProcessVote -- Already known valid vote";
+        LogPrint("gobject", "%s\n", ostr.str());
+        exception = CGovernanceException(ostr.str(), GOVERNANCE_EXCEPTION_NONE);
+        return false;
+    }
+
     if(!mnodeman.Has(vote.GetMasternodeOutpoint())) {
         std::ostringstream ostr;
         ostr << "CGovernanceObject::ProcessVote -- Masternode " << vote.GetMasternodeOutpoint().ToStringShort() << " not found";
@@ -193,9 +203,7 @@ bool CGovernanceObject::ProcessVote(CNode* pfrom,
     }
 
     voteInstance = vote_instance_t(vote.GetOutcome(), nVoteTimeUpdate, vote.GetTimestamp());
-    if(!fileVotes.HasVote(vote.GetHash())) {
     fileVotes.AddVote(vote);
-    }
     fDirtyCache = true;
     return true;
 }
@@ -510,19 +518,6 @@ bool CGovernanceObject::IsValidLocally(std::string& strError, bool& fMissingMast
             return false;
     }
 
-    /*
-        TODO
-
-        - There might be an issue with multisig in the coinbase on mainnet, we will add support for it in a future release.
-        - Post 12.2+ (test multisig coinbase transaction)
-    */
-
-    // 12.1 - todo - compile error
-    // if(address.IsPayToScriptHash()) {
-    //     strError = "Governance system - multisig is not currently supported";
-    //     return false;
-    // }
-
     return true;
 }
 
@@ -555,6 +550,12 @@ bool CGovernanceObject::IsCollateralValid(std::string& strError, bool& fMissingC
         return false;
     }
 
+    if(nBlockHash == uint256()) {
+        strError = strprintf("Collateral tx %s is not mined yet", txCollateral->ToString());
+        LogPrintf("CGovernanceObject::IsCollateralValid -- %s\n", strError);
+        return false;
+    }
+
     if(txCollateral->vout.size() < 1) {
         strError = strprintf("tx vout size less than 1 | %d", txCollateral->vout.size());
         LogPrintf("CGovernanceObject::IsCollateralValid -- %s\n", strError);

src/governance-votedb.cpp

@@ -20,8 +20,12 @@ CGovernanceObjectVoteFile::CGovernanceObjectVoteFile(const CGovernanceObjectVote
 
 void CGovernanceObjectVoteFile::AddVote(const CGovernanceVote& vote)
 {
+    uint256 nHash = vote.GetHash();
+    // make sure to never add/update already known votes
+    if (HasVote(nHash))
+        return;
     listVotes.push_front(vote);
-    mapVoteIndex[vote.GetHash()] = listVotes.begin();
+    mapVoteIndex.emplace(nHash, listVotes.begin());
     ++nMemoryVotes;
 }
 

src/masternodeman.cpp

@@ -208,7 +208,8 @@ void CMasternodeMan::CheckAndRemove(CConnman& connman)
                 bool fAsk = (nAskForMnbRecovery > 0) &&
                             masternodeSync.IsSynced() &&
                             it->second.IsNewStartRequired() &&
-                            !IsMnbRecoveryRequested(hash);
+                            !IsMnbRecoveryRequested(hash) &&
+                            !IsArgSet("-connect");
                 if(fAsk) {
                     // this mn is in a non-recoverable state and we haven't asked other nodes yet
                     std::set<CService> setRequested;

src/net.cpp

@@ -2412,6 +2412,12 @@ void CConnman::Interrupt()
             semAddnode->post();
         }
     }
+
+    if (semMasternodeOutbound) {
+        for (int i=0; i<MAX_OUTBOUND_MASTERNODE_CONNECTIONS; i++) {
+            semMasternodeOutbound->post();
+        }
+    }
 }
 
 void CConnman::Stop()
@@ -2429,10 +2435,6 @@ void CConnman::Stop()
     if (threadSocketHandler.joinable())
         threadSocketHandler.join();
 
-    if (semMasternodeOutbound)
-        for (int i=0; i<MAX_OUTBOUND_MASTERNODE_CONNECTIONS; i++)
-            semMasternodeOutbound->post();
-
     if (fAddressesInitialized)
     {
         DumpData();

src/wallet/wallet.cpp

@@ -3343,7 +3343,7 @@ bool CWallet::CreateCollateralTransaction(CMutableTransaction& txCollateral, std
         txCollateral.vout.push_back(CTxOut(0, CScript() << OP_RETURN));
     }
 
-    if(!SignSignature(*this, txdsinCollateral.prevPubKey, txCollateral, 0, int(SIGHASH_ALL|SIGHASH_ANYONECANPAY))) {
+    if (!SignSignature(*this, txdsinCollateral.prevPubKey, txCollateral, 0)) {
         strReason = "Unable to sign collateral transaction!";
         return false;
     }