backport: merge bitcoin#20222, #18766, #14033, #19486, #19607, #20291, partial #20187, #20228 (deglobalize addrman and feeEstimator)

src/addrman.h

@@ -268,8 +268,18 @@ class CAddrMan
     //! Select several addresses at once.
     void GetAddr_(std::vector<CAddress> &vAddr) EXCLUSIVE_LOCKS_REQUIRED(cs);
 
-    //! Mark an entry as currently-connected-to.
-    void Connected_(const CService &addr, int64_t nTime) EXCLUSIVE_LOCKS_REQUIRED(cs);
+    /** We have successfully connected to this peer. Calling this function
+     *  updates the CAddress's nTime, which is used in our IsTerrible()
+     *  decisions and gossiped to peers. Callers should be careful that updating
+     *  this information doesn't leak topology information to network spies.
+     *
+     *  net_processing calls this function when it *disconnects* from a peer to
+     *  not leak information about currently connected peers.
+     *
+     * @param[in]   addr     The address of the peer we were connected to
+     * @param[in]   nTime    The time that we were last connected to this peer
+     */
+    void Connected_(const CService& addr, int64_t nTime) EXCLUSIVE_LOCKS_REQUIRED(cs);
 
     //! Update an entry's service bits.
     void SetServices_(const CService &addr, ServiceFlags nServices) EXCLUSIVE_LOCKS_REQUIRED(cs);
@@ -710,7 +720,7 @@ class CAddrMan
         return vAddr;
     }
 
-    //! Mark an entry as currently-connected-to.
+    //! Outer function for Connected_()
     void Connected(const CService &addr, int64_t nTime = GetAdjustedTime())
     {
         LOCK(cs);

src/coinjoin/client.cpp

@@ -54,7 +54,6 @@ void CCoinJoinClientQueueManager::ProcessDSQueue(const CNode& peer, CDataStream&
     vRecv >> dsq;
 
     if (dsq.masternodeOutpoint.IsNull() && dsq.m_protxHash.IsNull()) {
-        LOCK(cs_main);
         Misbehaving(peer.GetId(), 100);
         return;
     }
@@ -64,7 +63,6 @@ void CCoinJoinClientQueueManager::ProcessDSQueue(const CNode& peer, CDataStream&
         if (auto dmn = mnList.GetValidMN(dsq.m_protxHash)) {
             dsq.masternodeOutpoint = dmn->collateralOutpoint;
         } else {
-            LOCK(cs_main);
             Misbehaving(peer.GetId(), 10);
             return;
         }
@@ -100,7 +98,6 @@ void CCoinJoinClientQueueManager::ProcessDSQueue(const CNode& peer, CDataStream&
     }
 
     if (!dsq.CheckSignature(dmn->pdmnState->pubKeyOperator.Get())) {
-        LOCK(cs_main);
         Misbehaving(peer.GetId(), 10);
         return;
     }
@@ -756,7 +753,7 @@ bool CCoinJoinClientManager::CheckAutomaticBackup()
 //
 // Passively run mixing in the background to mix funds based on the given configuration.
 //
-bool CCoinJoinClientSession::DoAutomaticDenominating(CTxMemPool& mempool, CConnman& connman, bool fDryRun)
+bool CCoinJoinClientSession::DoAutomaticDenominating(CConnman& connman, CBlockPolicyEstimator& fee_estimator, CTxMemPool& mempool, bool fDryRun)
 {
     if (fMasternodeMode) return false; // no client-side mixing on masternodes
     if (nState != POOL_STATE_IDLE) return false;
@@ -875,12 +872,12 @@ bool CCoinJoinClientSession::DoAutomaticDenominating(CTxMemPool& mempool, CConnm
         // there are funds to denominate and denominated balance does not exceed
         // max amount to mix yet.
         if (nBalanceAnonimizableNonDenom >= nValueMin + CCoinJoin::GetCollateralAmount() && nBalanceToDenominate > 0) {
-            CreateDenominated(nBalanceToDenominate);
+            CreateDenominated(fee_estimator, nBalanceToDenominate);
         }
 
         //check if we have the collateral sized inputs
         if (!mixingWallet.HasCollateralInputs()) {
-            return !mixingWallet.HasCollateralInputs(false) && MakeCollateralAmounts();
+            return !mixingWallet.HasCollateralInputs(false) && MakeCollateralAmounts(fee_estimator);
         }
 
         if (nSessionID) {
@@ -936,7 +933,7 @@ bool CCoinJoinClientSession::DoAutomaticDenominating(CTxMemPool& mempool, CConnm
     return false;
 }
 
-bool CCoinJoinClientManager::DoAutomaticDenominating(CTxMemPool& mempool, CConnman& connman, bool fDryRun)
+bool CCoinJoinClientManager::DoAutomaticDenominating(CConnman& connman, CBlockPolicyEstimator& fee_estimator, CTxMemPool& mempool, bool fDryRun)
 {
     if (fMasternodeMode) return false; // no client-side mixing on masternodes
     if (!CCoinJoinClientOptions::IsEnabled() || !IsMixing()) return false;
@@ -978,7 +975,7 @@ bool CCoinJoinClientManager::DoAutomaticDenominating(CTxMemPool& mempool, CConnm
             return false;
         }
 
-        fResult &= session.DoAutomaticDenominating(mempool, connman, fDryRun);
+        fResult &= session.DoAutomaticDenominating(connman, fee_estimator, mempool, fDryRun);
     }
 
     return fResult;
@@ -1372,7 +1369,7 @@ bool CCoinJoinClientSession::PrepareDenominate(int nMinRounds, int nMaxRounds, s
 }
 
 // Create collaterals by looping through inputs grouped by addresses
-bool CCoinJoinClientSession::MakeCollateralAmounts()
+bool CCoinJoinClientSession::MakeCollateralAmounts(const CBlockPolicyEstimator& fee_estimator)
 {
     if (!CCoinJoinClientOptions::IsEnabled()) return false;
 
@@ -1395,13 +1392,13 @@ bool CCoinJoinClientSession::MakeCollateralAmounts()
 
     // First try to use only non-denominated funds
     for (const auto& item : vecTally) {
-        if (!MakeCollateralAmounts(item, false)) continue;
+        if (!MakeCollateralAmounts(fee_estimator, item, false)) continue;
         return true;
     }
 
     // There should be at least some denominated funds we should be able to break in pieces to continue mixing
     for (const auto& item : vecTally) {
-        if (!MakeCollateralAmounts(item, true)) continue;
+        if (!MakeCollateralAmounts(fee_estimator, item, true)) continue;
         return true;
     }
 
@@ -1411,7 +1408,7 @@ bool CCoinJoinClientSession::MakeCollateralAmounts()
 }
 
 // Split up large inputs or create fee sized inputs
-bool CCoinJoinClientSession::MakeCollateralAmounts(const CompactTallyItem& tallyItem, bool fTryDenominated)
+bool CCoinJoinClientSession::MakeCollateralAmounts(const CBlockPolicyEstimator& fee_estimator, const CompactTallyItem& tallyItem, bool fTryDenominated)
 {
     AssertLockHeld(mixingWallet.cs_wallet);
 
@@ -1434,7 +1431,7 @@ bool CCoinJoinClientSession::MakeCollateralAmounts(const CompactTallyItem& tally
         return false;
     }
 
-    CTransactionBuilder txBuilder(pwallet, tallyItem);
+    CTransactionBuilder txBuilder(pwallet, tallyItem, fee_estimator);
 
     LogPrint(BCLog::COINJOIN, "CCoinJoinClientSession::%s -- Start %s\n", __func__, txBuilder.ToString());
 
@@ -1553,7 +1550,7 @@ bool CCoinJoinClientSession::CreateCollateralTransaction(CMutableTransaction& tx
 }
 
 // Create denominations by looping through inputs grouped by addresses
-bool CCoinJoinClientSession::CreateDenominated(CAmount nBalanceToDenominate)
+bool CCoinJoinClientSession::CreateDenominated(CBlockPolicyEstimator& fee_estimator, CAmount nBalanceToDenominate)
 {
     if (!CCoinJoinClientOptions::IsEnabled()) return false;
 
@@ -1577,7 +1574,7 @@ bool CCoinJoinClientSession::CreateDenominated(CAmount nBalanceToDenominate)
     bool fCreateMixingCollaterals = !mixingWallet.HasCollateralInputs();
 
     for (const auto& item : vecTally) {
-        if (!CreateDenominated(nBalanceToDenominate, item, fCreateMixingCollaterals)) continue;
+        if (!CreateDenominated(fee_estimator, nBalanceToDenominate, item, fCreateMixingCollaterals)) continue;
         return true;
     }
 
@@ -1586,7 +1583,7 @@ bool CCoinJoinClientSession::CreateDenominated(CAmount nBalanceToDenominate)
 }
 
 // Create denominations
-bool CCoinJoinClientSession::CreateDenominated(CAmount nBalanceToDenominate, const CompactTallyItem& tallyItem, bool fCreateMixingCollaterals)
+bool CCoinJoinClientSession::CreateDenominated(CBlockPolicyEstimator& fee_estimator, CAmount nBalanceToDenominate, const CompactTallyItem& tallyItem, bool fCreateMixingCollaterals)
 {
     AssertLockHeld(mixingWallet.cs_wallet);
 
@@ -1604,7 +1601,7 @@ bool CCoinJoinClientSession::CreateDenominated(CAmount nBalanceToDenominate, con
         return false;
     }
 
-    CTransactionBuilder txBuilder(pwallet, tallyItem);
+    CTransactionBuilder txBuilder(pwallet, tallyItem, fee_estimator);
 
     LogPrint(BCLog::COINJOIN, "CCoinJoinClientSession::%s -- Start %s\n", __func__, txBuilder.ToString());
 
@@ -1815,7 +1812,7 @@ void CCoinJoinClientQueueManager::DoMaintenance()
     CheckQueue();
 }
 
-void CCoinJoinClientManager::DoMaintenance(CTxMemPool& mempool, CConnman& connman)
+void CCoinJoinClientManager::DoMaintenance(CConnman& connman, CBlockPolicyEstimator& fee_estimator, CTxMemPool& mempool)
 {
     if (!CCoinJoinClientOptions::IsEnabled()) return;
     if (m_mn_sync == nullptr) return;
@@ -1830,7 +1827,7 @@ void CCoinJoinClientManager::DoMaintenance(CTxMemPool& mempool, CConnman& connma
     CheckTimeout();
     ProcessPendingDsaRequest(connman);
     if (nDoAutoNextRun == nTick) {
-        DoAutomaticDenominating(mempool, connman);
+        DoAutomaticDenominating(connman, fee_estimator, mempool);
         nDoAutoNextRun = nTick + COINJOIN_AUTO_TIMEOUT_MIN + GetRandInt(COINJOIN_AUTO_TIMEOUT_MAX - COINJOIN_AUTO_TIMEOUT_MIN);
     }
 }
@@ -1867,14 +1864,13 @@ void CCoinJoinClientManager::GetJsonInfo(UniValue& obj) const
     obj.pushKV("sessions",  arrSessions);
 }
 
-void DoCoinJoinMaintenance(CTxMemPool& mempool, CConnman& connman)
+void DoCoinJoinMaintenance(CConnman& connman, CBlockPolicyEstimator& fee_estimator, CTxMemPool& mempool)
 {
     if (coinJoinClientQueueManager != nullptr) {
         coinJoinClientQueueManager->DoMaintenance();
     }
 
     for (const auto& pair : coinJoinClientManagers) {
-        pair.second->DoMaintenance(mempool, connman);
+        pair.second->DoMaintenance(connman, fee_estimator, mempool);
     }
 }
-

src/coinjoin/client.h

@@ -18,6 +18,7 @@ using CDeterministicMNCPtr = std::shared_ptr<const CDeterministicMN>;
 class CCoinJoinClientManager;
 class CCoinJoinClientQueueManager;
 
+class CBlockPolicyEstimator;
 class CConnman;
 class CNode;
 class CTxMemPool;
@@ -88,12 +89,12 @@ class CCoinJoinClientSession : public CCoinJoinBaseSession
     CWallet& mixingWallet;
 
     /// Create denominations
-    bool CreateDenominated(CAmount nBalanceToDenominate);
-    bool CreateDenominated(CAmount nBalanceToDenominate, const CompactTallyItem& tallyItem, bool fCreateMixingCollaterals);
+    bool CreateDenominated(CBlockPolicyEstimator& fee_estimator, CAmount nBalanceToDenominate);
+    bool CreateDenominated(CBlockPolicyEstimator& fee_estimator, CAmount nBalanceToDenominate, const CompactTallyItem& tallyItem, bool fCreateMixingCollaterals);
 
     /// Split up large inputs or make fee sized inputs
-    bool MakeCollateralAmounts();
-    bool MakeCollateralAmounts(const CompactTallyItem& tallyItem, bool fTryDenominated);
+    bool MakeCollateralAmounts(const CBlockPolicyEstimator& fee_estimator);
+    bool MakeCollateralAmounts(const CBlockPolicyEstimator& fee_estimator, const CompactTallyItem& tallyItem, bool fTryDenominated);
 
     bool CreateCollateralTransaction(CMutableTransaction& txCollateral, std::string& strReason);
 
@@ -138,7 +139,7 @@ class CCoinJoinClientSession : public CCoinJoinBaseSession
     bool GetMixingMasternodeInfo(CDeterministicMNCPtr& ret) const;
 
     /// Passively run mixing in the background according to the configuration in settings
-    bool DoAutomaticDenominating(CTxMemPool& mempool, CConnman& connman, bool fDryRun = false) LOCKS_EXCLUDED(cs_coinjoin);
+    bool DoAutomaticDenominating(CConnman& connman, CBlockPolicyEstimator& fee_estimator, CTxMemPool& mempool, bool fDryRun = false) LOCKS_EXCLUDED(cs_coinjoin);
 
     /// As a client, submit part of a future mixing transaction to a Masternode to start the process
     bool SubmitDenominate(CConnman& connman);
@@ -221,7 +222,7 @@ class CCoinJoinClientManager
     bool GetMixingMasternodesInfo(std::vector<CDeterministicMNCPtr>& vecDmnsRet) const LOCKS_EXCLUDED(cs_deqsessions);
 
     /// Passively run mixing in the background according to the configuration in settings
-    bool DoAutomaticDenominating(CTxMemPool& mempool, CConnman& connman, bool fDryRun = false) LOCKS_EXCLUDED(cs_deqsessions);
+    bool DoAutomaticDenominating(CConnman& connman, CBlockPolicyEstimator& fee_estimator, CTxMemPool& mempool, bool fDryRun = false) LOCKS_EXCLUDED(cs_deqsessions);
 
     bool TrySubmitDenominate(const CService& mnAddr, CConnman& connman) LOCKS_EXCLUDED(cs_deqsessions);
     bool MarkAlreadyJoinedQueueAsTried(CCoinJoinQueue& dsq) const LOCKS_EXCLUDED(cs_deqsessions);
@@ -237,12 +238,12 @@ class CCoinJoinClientManager
 
     void UpdatedBlockTip(const CBlockIndex* pindex);
 
-    void DoMaintenance(CTxMemPool& mempool, CConnman& connman);
+    void DoMaintenance(CConnman& connman, CBlockPolicyEstimator& fee_estimator, CTxMemPool& mempool);
 
     void GetJsonInfo(UniValue& obj) const LOCKS_EXCLUDED(cs_deqsessions);
 };
 
 
-void DoCoinJoinMaintenance(CTxMemPool& mempool, CConnman& connman);
+void DoCoinJoinMaintenance(CConnman& connman, CBlockPolicyEstimator& fee_estimator, CTxMemPool& mempool);
 
 #endif // BITCOIN_COINJOIN_CLIENT_H

src/coinjoin/server.cpp

@@ -113,7 +113,6 @@ void CCoinJoinServer::ProcessDSQUEUE(const CNode& peer, CDataStream& vRecv)
     vRecv >> dsq;
 
     if (dsq.masternodeOutpoint.IsNull() && dsq.m_protxHash.IsNull()) {
-        LOCK(cs_main);
         Misbehaving(peer.GetId(), 100);
         return;
     }
@@ -123,7 +122,6 @@ void CCoinJoinServer::ProcessDSQUEUE(const CNode& peer, CDataStream& vRecv)
         if (auto dmn = mnList.GetValidMN(dsq.m_protxHash)) {
             dsq.masternodeOutpoint = dmn->collateralOutpoint;
         } else {
-            LOCK(cs_main);
             Misbehaving(peer.GetId(), 10);
             return;
         }
@@ -159,7 +157,6 @@ void CCoinJoinServer::ProcessDSQUEUE(const CNode& peer, CDataStream& vRecv)
     }
 
     if (!dsq.CheckSignature(dmn->pdmnState->pubKeyOperator.Get())) {
-        LOCK(cs_main);
         Misbehaving(peer.GetId(), 10);
         return;
     }

src/coinjoin/util.cpp

@@ -107,15 +107,15 @@ bool CTransactionBuilderOutput::UpdateAmount(const CAmount nNewAmount)
     return true;
 }
 
-CTransactionBuilder::CTransactionBuilder(std::shared_ptr<CWallet> pwalletIn, const CompactTallyItem& tallyItemIn) :
+CTransactionBuilder::CTransactionBuilder(std::shared_ptr<CWallet> pwalletIn, const CompactTallyItem& tallyItemIn, const CBlockPolicyEstimator& fee_estimator) :
     pwallet(pwalletIn),
     dummyReserveDestination(pwalletIn.get()),
     tallyItem(tallyItemIn)
 {
     // Generate a feerate which will be used to consider if the remainder is dust and will go into fees or not
     coinControl.m_discard_feerate = ::GetDiscardRate(*pwallet);
     // Generate a feerate which will be used by calculations of this class and also by CWallet::CreateTransaction
-    coinControl.m_feerate = std::max(::feeEstimator.estimateSmartFee(int(pwallet->m_confirm_target), nullptr, true), pwallet->m_pay_tx_fee);
+    coinControl.m_feerate = std::max(fee_estimator.estimateSmartFee(int(pwallet->m_confirm_target), nullptr, true), pwallet->m_pay_tx_fee);
     // Change always goes back to origin
     coinControl.destChange = tallyItemIn.txdest;
     // Only allow tallyItems inputs for tx creation

src/coinjoin/util.h

@@ -7,6 +7,7 @@
 
 #include <wallet/wallet.h>
 
+class CBlockPolicyEstimator;
 class CTransactionBuilder;
 struct bilingual_str;
 
@@ -100,7 +101,7 @@ class CTransactionBuilder
     friend class CTransactionBuilderOutput;
 
 public:
-    CTransactionBuilder(std::shared_ptr<CWallet> pwalletIn, const CompactTallyItem& tallyItemIn);
+    CTransactionBuilder(std::shared_ptr<CWallet> pwalletIn, const CompactTallyItem& tallyItemIn, const CBlockPolicyEstimator& fee_estimator);
     ~CTransactionBuilder();
     /// Check it would be possible to add a single output with the amount nAmount. Returns true if its possible and false if not.
     bool CouldAddOutput(CAmount nAmountOutput) const;

src/context.h

@@ -10,6 +10,7 @@
 
 class ChainstateManager;
 class CTxMemPool;
+class CBlockPolicyEstimator;
 struct LLMQContext;
 struct NodeContext;
 struct WalletContext;
@@ -19,6 +20,7 @@ using CoreContext = std::variant<std::nullopt_t,
                                  std::reference_wrapper<WalletContext>,
                                  std::reference_wrapper<CTxMemPool>,
                                  std::reference_wrapper<ChainstateManager>,
+                                 std::reference_wrapper<CBlockPolicyEstimator>,
                                  std::reference_wrapper<LLMQContext>>;
 
 template<typename T>

src/evo/mnauth.cpp

@@ -70,34 +70,29 @@ void CMNAuth::ProcessMessage(CNode& peer, CConnman& connman, std::string_view ms
 
     // only one MNAUTH allowed
     if (!peer.GetVerifiedProRegTxHash().IsNull()) {
-        LOCK(cs_main);
         Misbehaving(peer.GetId(), 100, "duplicate mnauth");
         return;
     }
 
     if ((~peer.nServices) & (NODE_NETWORK | NODE_BLOOM)) {
         // either NODE_NETWORK or NODE_BLOOM bit is missing in node's services
-        LOCK(cs_main);
         Misbehaving(peer.GetId(), 100, "mnauth from a node with invalid services");
         return;
     }
 
     if (mnauth.proRegTxHash.IsNull()) {
-        LOCK(cs_main);
         Misbehaving(peer.GetId(), 100, "empty mnauth proRegTxHash");
         return;
     }
 
     if (!mnauth.sig.IsValid()) {
-        LOCK(cs_main);
         Misbehaving(peer.GetId(), 100, "invalid mnauth signature");
         return;
     }
 
     const auto mnList = deterministicMNManager->GetListAtChainTip();
     const auto dmn = mnList.GetMN(mnauth.proRegTxHash);
     if (!dmn) {
-        LOCK(cs_main);
         // in case node was unlucky and not up to date, just let it be connected as a regular node, which gives it
         // a chance to get up-to-date and thus realize that it's not a MN anymore. We still give it a
         // low DoS score.
@@ -122,7 +117,6 @@ void CMNAuth::ProcessMessage(CNode& peer, CConnman& connman, std::string_view ms
     LogPrint(BCLog::NET_NETCONN, "CMNAuth::%s -- constructed signHash for nVersion %d, peer=%d\n", __func__, peer.nVersion, peer.GetId());
 
     if (!mnauth.sig.VerifyInsecure(dmn->pdmnState->pubKeyOperator.Get(), signHash)) {
-        LOCK(cs_main);
         // Same as above, MN seems to not know its fate yet, so give it a chance to update. If this is a
         // malicious node (DoSing us), it'll get banned soon.
         Misbehaving(peer.GetId(), 10, "mnauth signature verification failed");