backport: v24.0.x backports for v24.0.1

depends/packages/miniupnpc.mk

@@ -1,6 +1,6 @@
 package=miniupnpc
 $(package)_version=2.2.7
-$(package)_download_path=https://miniupnp.tuxfamily.org/files/
+$(package)_download_path=http://miniupnp.free.fr/files/
 $(package)_file_name=$(package)-$($(package)_version).tar.gz
 $(package)_sha256_hash=b0c3a27056840fd0ec9328a5a9bac3dc5e0ec6d2e8733349cf577b0aa1e70ac1
 $(package)_patches=dont_leak_info.patch cmake_get_src_addr.patch fix_windows_snprintf.patch

src/core_write.cpp

@@ -289,11 +289,11 @@ void TxToUniv(const CTransaction& tx, const uint256& block_hash, UniValue& entry
     }
 
     if (tx.nType == TRANSACTION_PROVIDER_REGISTER) {
-        if (const auto opt_proTx = GetTxPayload<CProRegTx>(tx)) {
+        if (const auto opt_proTx = GetTxPayload<CProRegTx>(tx); opt_proTx && opt_proTx->netInfo) {
             entry.pushKV("proRegTx", opt_proTx->ToJson());
         }
     } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_SERVICE) {
-        if (const auto opt_proTx = GetTxPayload<CProUpServTx>(tx)) {
+        if (const auto opt_proTx = GetTxPayload<CProUpServTx>(tx); opt_proTx && opt_proTx->netInfo) {
             entry.pushKV("proUpServTx", opt_proTx->ToJson());
         }
     } else if (tx.nType == TRANSACTION_PROVIDER_UPDATE_REGISTRAR) {

src/evo/specialtxman.cpp

@@ -1093,6 +1093,11 @@ bool CSpecialTxProcessor::UndoSpecialTxsInBlock(const Chainstate& chainstate, co
     return true;
 }
 
+void CSpecialTxProcessor::RemoveMineableCommitmentsForBlock(const CBlock& block, gsl::not_null<const CBlockIndex*> pindex)
+{
+    m_qblockman.RemoveMineableCommitmentsForBlock(block, pindex);
+}
+
 bool CSpecialTxProcessor::CheckCreditPoolDiffForBlock(const CBlock& block, const CBlockIndex* pindex, const CCbTx& cbTx,
                                                       CAmount blockSubsidy, BlockValidationState& state)
 {

src/evo/specialtxman.h

@@ -97,6 +97,8 @@ class CSpecialTxProcessor
         EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
     bool UndoSpecialTxsInBlock(const Chainstate& chainstate, const CBlock& block, const CBlockIndex* pindex, MNListUpdates& updatesRet)
         EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
+    void RemoveMineableCommitmentsForBlock(const CBlock& block, gsl::not_null<const CBlockIndex*> pindex)
+        EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
 
 
     // the returned list will not contain the correct block hash (we can't know it yet as the coinbase TX is not updated yet)

src/governance/governance.cpp

@@ -1190,10 +1190,14 @@ void CGovernanceManager::RemoveInvalidVotes()
 
     std::vector<COutPoint> changedKeyMNs;
     for (const auto& [internalId, pdmnState] : diff.updatedMNs) {
-        auto oldDmn = lastMNListForVotingKeys->GetMNByInternalId(internalId);
-        // BuildDiff will construct itself with MNs that we already have knowledge
-        // of, meaning that fetch operations should never fail.
-        assert(oldDmn);
+        auto newDmn = tip_mn_list.GetMNByInternalId(internalId);
+        if (!Assume(newDmn)) {
+            continue;
+        }
+        auto oldDmn = lastMNListForVotingKeys->GetMN(newDmn->proTxHash);
+        if (!Assume(oldDmn)) {
+            continue;
+        }
         if ((pdmnState.fields & CDeterministicMNStateDiff::Field_keyIDVoting) && pdmnState.state.keyIDVoting != oldDmn->pdmnState->keyIDVoting) {
             changedKeyMNs.emplace_back(oldDmn->collateralOutpoint);
         } else if ((pdmnState.fields & CDeterministicMNStateDiff::Field_pubKeyOperator) && pdmnState.state.pubKeyOperator != oldDmn->pdmnState->pubKeyOperator) {
@@ -1202,9 +1206,9 @@ void CGovernanceManager::RemoveInvalidVotes()
     }
     for (const auto& id : diff.removedMns) {
         auto oldDmn = lastMNListForVotingKeys->GetMNByInternalId(id);
-        // BuildDiff will construct itself with MNs that we already have knowledge
-        // of, meaning that fetch operations should never fail.
-        assert(oldDmn);
+        if (!Assume(oldDmn)) {
+            continue;
+        }
         changedKeyMNs.emplace_back(oldDmn->collateralOutpoint);
     }
 

src/llmq/blockprocessor.cpp

@@ -9,6 +9,7 @@
 #include <llmq/commitment.h>
 #include <llmq/options.h>
 #include <llmq/utils.h>
+#include <util/check.h>
 #include <util/helpers.h>
 #include <util/std23.h>
 
@@ -254,8 +255,7 @@ bool CQuorumBlockProcessor::ProcessBlock(Chainstate& chainstate, const CBlock& b
 
     // ProcessCommitment accepts a commitment identical to the stored one, so
     // rotation's multiple commitments per type need an explicit per-block check.
-    // It runs before any commitment is processed, because ProcessCommitment
-    // updates in-memory state that a rejected block would not restore.
+    // It runs before the signature checks, so a duplicate is rejected cheaply.
     // Null commitments stay exempt: they were never subject to bad-qc-dup.
     std::set<std::pair<Consensus::LLMQType, uint256>> seen_quorums;
     for (const auto& [_, qc] : qcs) {
@@ -445,12 +445,7 @@ bool CQuorumBlockProcessor::ProcessCommitment(Chainstate& chainstate, int nHeigh
     // invalidation should not depend on that.
     DropQcHashesCache();
 
-    {
-        LOCK(minableCommitmentsCs);
-        mapMinedCommitmentBlockCache.erase(qc.llmqType, qc.quorumHash);
-        minableCommitmentsByQuorum.erase(cacheKey);
-        minableCommitments.erase(::SerializeHash(qc));
-    }
+    WITH_LOCK(minableCommitmentsCs, mapMinedCommitmentBlockCache.erase(qc.llmqType, qc.quorumHash));
 
     LogPrint(BCLog::LLMQ, "%s -- processed commitment from block. type=%d, quorumIndex=%d, quorumHash=%s\n", __func__,
              std23::to_underlying(qc.llmqType), qc.quorumIndex, quorumHash.ToString());
@@ -561,6 +556,23 @@ bool CQuorumBlockProcessor::UndoBlock(const Chainstate& chainstate, const CBlock
     return true;
 }
 
+void CQuorumBlockProcessor::RemoveMineableCommitmentsForBlock(const CBlock& block, gsl::not_null<const CBlockIndex*> pindex)
+{
+    AssertLockHeld(::cs_main);
+
+    std::multimap<Consensus::LLMQType, CFinalCommitment> qcs;
+    if (BlockValidationState dummy; !Assume(GetCommitmentsFromBlock(block, pindex, qcs, dummy))) {
+        return;
+    }
+
+    LOCK(minableCommitmentsCs);
+    for (const auto& [_, qc] : qcs) {
+        if (qc.IsNull()) continue;
+        minableCommitmentsByQuorum.erase(std::make_pair(qc.llmqType, qc.quorumHash));
+        minableCommitments.erase(::SerializeHash(qc));
+    }
+}
+
 bool CQuorumBlockProcessor::GetCommitmentsFromBlock(const CBlock& block, gsl::not_null<const CBlockIndex*> pindex, std::multimap<Consensus::LLMQType, CFinalCommitment>& ret, BlockValidationState& state)
 {
     AssertLockHeld(::cs_main);

src/llmq/blockprocessor.h

@@ -108,6 +108,9 @@ class CQuorumBlockProcessor
                       bool fJustCheck, bool fBLSChecks) EXCLUSIVE_LOCKS_REQUIRED(::cs_main, !minableCommitmentsCs, !m_qc_hashes_cache_mutex);
     bool UndoBlock(const Chainstate& chainstate, const CBlock& block, gsl::not_null<const CBlockIndex*> pindex)
         EXCLUSIVE_LOCKS_REQUIRED(::cs_main, !minableCommitmentsCs, !m_qc_hashes_cache_mutex);
+    //! Drop the commitments mined by a block that has been fully connected from the mineable pool.
+    void RemoveMineableCommitmentsForBlock(const CBlock& block, gsl::not_null<const CBlockIndex*> pindex)
+        EXCLUSIVE_LOCKS_REQUIRED(::cs_main, !minableCommitmentsCs);
 
     //! it returns hash of commitment if it should be relay, otherwise nullopt
     std::optional<CInv> AddMineableCommitment(const CFinalCommitment& fqc) EXCLUSIVE_LOCKS_REQUIRED(!minableCommitmentsCs);

src/llmq/dkgsessionhandler.cpp

@@ -52,7 +52,12 @@ void CDKGPendingMessages::PushPendingMessage(NodeId from, const uint256& sender_
     count++;
 
     seenMessages.emplace(hash);
-    pendingMessages.emplace_back(from, std::move(pm));
+    if (from == -1) {
+        // Our own message must not wait behind a backlog of peer messages before it is relayed
+        pendingMessages.emplace_front(from, std::move(pm));
+    } else {
+        pendingMessages.emplace_back(from, std::move(pm));
+    }
 }
 
 std::list<CDKGPendingMessages::BinaryMessage> CDKGPendingMessages::PopPendingMessages(size_t maxCount)

src/llmq/dkgsessionhandler.h

@@ -71,7 +71,8 @@ class CDKGPendingMessages
      * Enqueue a serialized DKG message under @p from with content hash @p hash.
      * @p sender_protx keys the per-proTx quota: the sender's MNAuth-verified
      * proTxHash for remote messages, or this node's own for messages it
-     * produced itself (@p from == -1). Drops the message silently on quota
+     * produced itself (@p from == -1), which go to the front of the queue.
+     * Drops the message silently on quota
      * overflow or duplicate hash; quota-dropped messages are not marked seen,
      * so another peer with budget can re-deliver them. Caller is responsible
      * for hashing the payload and (for real peers) routing the erase-request

src/llmq/dkgsessionmgr.cpp

@@ -130,11 +130,22 @@ void CDKGSessionManager::WriteVerifiedVvecContribution(Consensus::LLMQType llmqT
         s << CBLSPublicKeyVersionWrapper(pubkey, false);
     }
     db->Write(std::make_tuple(DB_VVEC, llmqType, pQuorumBaseBlockIndex->GetBlockHash(), proTxHash), Span{s});
+
+    // Reading the vvec back at commit time costs a point decompression and subgroup check per coefficient
+    LOCK(contributionsCacheCs);
+    auto& entry = contributionsCache[{llmqType, pQuorumBaseBlockIndex->GetBlockHash(), proTxHash}];
+    entry.entryTime = SteadyClock::now();
+    entry.vvec = vvec;
 }
 
 void CDKGSessionManager::WriteVerifiedSkContribution(Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex, const uint256& proTxHash, const CBLSSecretKey& skContribution)
 {
     db->Write(std::make_tuple(DB_SKCONTRIB, llmqType, pQuorumBaseBlockIndex->GetBlockHash(), proTxHash), skContribution);
+
+    LOCK(contributionsCacheCs);
+    if (auto it = contributionsCache.find({llmqType, pQuorumBaseBlockIndex->GetBlockHash(), proTxHash}); it != contributionsCache.end()) {
+        it->second.skContribution = skContribution;
+    }
 }
 
 void CDKGSessionManager::WriteEncryptedContributions(Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex, const uint256& proTxHash, const CBLSIESMultiRecipientObjects<CBLSSecretKey>& contributions)
@@ -228,13 +239,10 @@ void CDKGSessionManager::CleanupCache() const
 {
     LOCK(contributionsCacheCs);
     const auto curTime = SteadyClock::now();
-    for (auto it = contributionsCache.begin(); it != contributionsCache.end(); ) {
-        if (curTime - it->second.entryTime > MAX_CONTRIBUTION_CACHE_TIME) {
-            it = contributionsCache.erase(it);
-        } else {
-            ++it;
-        }
-    }
+    // A vvec still referenced elsewhere (by its DKG session) costs nothing to keep and is expensive to reload
+    std::erase_if(contributionsCache, [&](const auto& entry) {
+        return entry.second.vvec.use_count() == 1 && curTime - entry.second.entryTime > MAX_CONTRIBUTION_CACHE_TIME;
+    });
 }
 
 void CDKGSessionManager::CleanupOldContributions() const

src/llmq/dkgsessionmgr.h

@@ -140,8 +140,10 @@ class CDKGSessionManager
     bool GetPrematureCommitment(const uint256& hash, CDKGPrematureCommitment& ret) const;
 
     // Contributions are written while in the DKG
-    void WriteVerifiedVvecContribution(Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex, const uint256& proTxHash, const BLSVerificationVectorPtr& vvec);
-    void WriteVerifiedSkContribution(Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex, const uint256& proTxHash, const CBLSSecretKey& skContribution);
+    void WriteVerifiedVvecContribution(Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex, const uint256& proTxHash, const BLSVerificationVectorPtr& vvec)
+        EXCLUSIVE_LOCKS_REQUIRED(!contributionsCacheCs);
+    void WriteVerifiedSkContribution(Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex, const uint256& proTxHash, const CBLSSecretKey& skContribution)
+        EXCLUSIVE_LOCKS_REQUIRED(!contributionsCacheCs);
     bool GetVerifiedContributions(Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex,
                                   const std::vector<bool>& validMembers, std::vector<uint16_t>& memberIndexesRet,
                                   std::vector<BLSVerificationVectorPtr>& vvecsRet,

src/llmq/ehf_signals.cpp

@@ -94,7 +94,7 @@ void CEHFSignalsHandler::trySignEHFSignal(int bit, const CBlockIndex* const pind
     const uint256 msgHash = mnhfPayload.PrepareTx().GetHash();
 
     WITH_LOCK(cs, ids.insert(requestId));
-    shareman.AsyncSignIfMember(llmqType, requestId, msgHash, quorum->qc->quorumHash, false, true);
+    shareman.AsyncSignIfMember(llmqType, requestId, msgHash, quorum->qc->quorumHash, true, true);
 }
 
 RecoveredSigResult CEHFSignalsHandler::HandleNewRecoveredSig(const CRecoveredSig& recoveredSig)

src/net.cpp

@@ -1771,6 +1771,7 @@ std::pair<size_t, bool> CConnman::SocketSendData(CNode& node) const
                     LogPrint(BCLog::NET, "socket send error for peer=%d: %s\n", node.GetId(), NetworkErrorString(nErr));
                     node.fDisconnect = true;
                 }
+                node.fCanSendData = false;
             }
             break;
         }

src/qt/mnsharesession.cpp

@@ -104,19 +104,6 @@ bool SameShare(const MnShareSession::Share& a, const MnShareSession::Share& b)
            a.rewardAddress == b.rewardAddress;
 }
 
-bool SameContribution(const MnShareSession::Contribution& a, const MnShareSession::Contribution& b)
-{
-    if (a.inputs.size() != b.inputs.size() || a.hasChange != b.hasChange) return false;
-    if (a.hasChange && (a.changeAddress != b.changeAddress || a.changeAmount != b.changeAmount)) return false;
-    for (size_t i = 0; i < a.inputs.size(); ++i) {
-        if (a.inputs[i].txid.compare(b.inputs[i].txid, Qt::CaseInsensitive) != 0 ||
-            a.inputs[i].vout != b.inputs[i].vout || a.inputs[i].sequence != b.inputs[i].sequence) {
-            return false;
-        }
-    }
-    return true;
-}
-
 bool IsTxidHex(const QString& txid)
 {
     return txid.size() == 64 && IsHex(txid.toStdString());
@@ -191,6 +178,19 @@ bool DecodeSharedProTx(const QString& protx_hex, CMutableTransaction& tx, CProRe
 
 } // anonymous namespace
 
+bool MnShareSession::SameContribution(const Contribution& a, const Contribution& b)
+{
+    if (a.inputs.size() != b.inputs.size() || a.hasChange != b.hasChange) return false;
+    if (a.hasChange && (a.changeAddress != b.changeAddress || a.changeAmount != b.changeAmount)) return false;
+    for (size_t i = 0; i < a.inputs.size(); ++i) {
+        if (a.inputs[i].txid.compare(b.inputs[i].txid, Qt::CaseInsensitive) != 0 ||
+            a.inputs[i].vout != b.inputs[i].vout || a.inputs[i].sequence != b.inputs[i].sequence) {
+            return false;
+        }
+    }
+    return true;
+}
+
 namespace {
 //! U+202F NARROW NO-BREAK SPACE. BitcoinUnits groups thousands with a plain
 //! thin space, which a layout may break a number across.

src/qt/mnsharesession.h

@@ -313,6 +313,9 @@ class MnShareSession
 
     //! Whether a row with `share`'s owner address has the same amount and addresses here
     bool hasShare(const Share& share) const;
+    //! Same inputs, in order and with their sequences, and the same change.
+    //! Labels and changeIndex are ignored.
+    static bool SameContribution(const Contribution& a, const Contribution& b);
     //! Whether a contribution with `contribution`'s label spends the same coins and pays the same change here
     bool hasContribution(const Contribution& contribution) const;