Merge #6447: [v22.0.x] backport: v22.0.0 rc.3

c7b0d80 Merge #6441: fix: hold wallet shared pointer in CJ Manager/Sessions to prevent concurrent unload (pasta)
c074e09 Merge #6444: fix: add platform transfer to "most common" filter (pasta)
cb04114 Merge #6442: fix: coin selection with `include_unsafe` option should respect `nCoinType` (pasta)
db5b53a Merge #6434: fix: early EHF and buried EHF are indistinguish (pasta)

Pull request description:

  ## Issue being fixed or feature implemented
  See commits; backports for rc.3

  ## What was done?

  ## How Has This Been Tested?
  built locally

  ## Breaking Changes

  ## Checklist:
    _Go over all the following points, and put an `x` in all the boxes that apply._
  - [ ] I have performed a self-review of my own code
  - [ ] I have commented my code, particularly in hard-to-understand areas
  - [ ] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  kwvg:
    ACK c7b0d80
  UdjinM6:
    utACK c7b0d80

Tree-SHA512: a64d6503a845ea86df8660d34cdf819c6fefcae5e82035bd8de40152f4f7d894cd1870315b791cca81e6d4db08d9929e4d1de3338a0338478072c9e6bb66952a

Author: PastaPastaPasta

src/coinjoin/client.cpp

@@ -96,7 +96,7 @@ class CoinJoinWalletManager {
         }
     }
 
-    void Add(CWallet& wallet);
+    void Add(const std::shared_ptr<CWallet>& wallet);
     void DoMaintenance();
 
     void Remove(const std::string& name);
@@ -138,7 +138,7 @@ class CoinJoinWalletManager {
 class CCoinJoinClientSession : public CCoinJoinBaseSession
 {
 private:
-    CWallet& m_wallet;
+    const std::shared_ptr<CWallet> m_wallet;
     CoinJoinWalletManager& m_walletman;
     CCoinJoinClientManager& m_clientman;
     CDeterministicMNManager& m_dmnman;
@@ -163,15 +163,15 @@ class CCoinJoinClientSession : public CCoinJoinBaseSession
     /// Create denominations
     bool CreateDenominated(CAmount nBalanceToDenominate);
     bool CreateDenominated(CAmount nBalanceToDenominate, const CompactTallyItem& tallyItem, bool fCreateMixingCollaterals)
-        EXCLUSIVE_LOCKS_REQUIRED(m_wallet.cs_wallet);
+        EXCLUSIVE_LOCKS_REQUIRED(m_wallet->cs_wallet);
 
     /// Split up large inputs or make fee sized inputs
     bool MakeCollateralAmounts();
     bool MakeCollateralAmounts(const CompactTallyItem& tallyItem, bool fTryDenominated)
-        EXCLUSIVE_LOCKS_REQUIRED(m_wallet.cs_wallet);
+        EXCLUSIVE_LOCKS_REQUIRED(m_wallet->cs_wallet);
 
     bool CreateCollateralTransaction(CMutableTransaction& txCollateral, std::string& strReason)
-        EXCLUSIVE_LOCKS_REQUIRED(m_wallet.cs_wallet);
+        EXCLUSIVE_LOCKS_REQUIRED(m_wallet->cs_wallet);
 
     bool JoinExistingQueue(CAmount nBalanceNeedsAnonymized, CConnman& connman);
     bool StartNewQueue(CAmount nBalanceNeedsAnonymized, CConnman& connman);
@@ -181,7 +181,7 @@ class CCoinJoinClientSession : public CCoinJoinBaseSession
     /// step 1: prepare denominated inputs and outputs
     bool PrepareDenominate(int nMinRounds, int nMaxRounds, std::string& strErrorRet, const std::vector<CTxDSIn>& vecTxDSIn,
                            std::vector<std::pair<CTxDSIn, CTxOut>>& vecPSInOutPairsRet, bool fDryRun = false)
-        EXCLUSIVE_LOCKS_REQUIRED(m_wallet.cs_wallet);
+        EXCLUSIVE_LOCKS_REQUIRED(m_wallet->cs_wallet);
     /// step 2: send denominated inputs and outputs prepared in step 1
     bool SendDenominate(const std::vector<std::pair<CTxDSIn, CTxOut> >& vecPSInOutPairsIn, CConnman& connman) EXCLUSIVE_LOCKS_REQUIRED(!cs_coinjoin);
 
@@ -200,7 +200,7 @@ class CCoinJoinClientSession : public CCoinJoinBaseSession
     void SetNull() override EXCLUSIVE_LOCKS_REQUIRED(cs_coinjoin);
 
 public:
-    explicit CCoinJoinClientSession(CWallet& wallet, CoinJoinWalletManager& walletman,
+    explicit CCoinJoinClientSession(const std::shared_ptr<CWallet>& wallet, CoinJoinWalletManager& walletman,
                                     CCoinJoinClientManager& clientman, CDeterministicMNManager& dmnman,
                                     CMasternodeMetaMan& mn_metaman, const CMasternodeSync& mn_sync,
                                     const std::unique_ptr<CCoinJoinClientQueueManager>& queueman, bool is_masternode);
@@ -267,7 +267,7 @@ class CCoinJoinClientQueueManager : public CCoinJoinBaseManager
 class CCoinJoinClientManager
 {
 private:
-    CWallet& m_wallet;
+    const std::shared_ptr<CWallet> m_wallet;
     CoinJoinWalletManager& m_walletman;
     CDeterministicMNManager& m_dmnman;
     CMasternodeMetaMan& m_mn_metaman;
@@ -306,11 +306,19 @@ class CCoinJoinClientManager
     CCoinJoinClientManager(CCoinJoinClientManager const&) = delete;
     CCoinJoinClientManager& operator=(CCoinJoinClientManager const&) = delete;
 
-    explicit CCoinJoinClientManager(CWallet& wallet, CoinJoinWalletManager& walletman, CDeterministicMNManager& dmnman,
-                                    CMasternodeMetaMan& mn_metaman, const CMasternodeSync& mn_sync,
+    explicit CCoinJoinClientManager(const std::shared_ptr<CWallet>& wallet, CoinJoinWalletManager& walletman,
+                                    CDeterministicMNManager& dmnman, CMasternodeMetaMan& mn_metaman,
+                                    const CMasternodeSync& mn_sync,
                                     const std::unique_ptr<CCoinJoinClientQueueManager>& queueman, bool is_masternode) :
-        m_wallet(wallet), m_walletman(walletman), m_dmnman(dmnman), m_mn_metaman(mn_metaman), m_mn_sync(mn_sync), m_queueman(queueman),
-        m_is_masternode{is_masternode} {}
+        m_wallet(wallet),
+        m_walletman(walletman),
+        m_dmnman(dmnman),
+        m_mn_metaman(mn_metaman),
+        m_mn_sync(mn_sync),
+        m_queueman(queueman),
+        m_is_masternode{is_masternode}
+    {
+    }
 
     void ProcessMessage(CNode& peer, CChainState& active_chainstate, CConnman& connman, const CTxMemPool& mempool, std::string_view msg_type, CDataStream& vRecv) EXCLUSIVE_LOCKS_REQUIRED(!cs_deqsessions);
 

src/coinjoin/client.h

@@ -67,10 +67,7 @@ class CoinJoinLoaderImpl : public interfaces::CoinJoin::Loader
     explicit CoinJoinLoaderImpl(CoinJoinWalletManager& walletman)
         : m_walletman(walletman) {}
 
-    void AddWallet(CWallet& wallet) override
-    {
-        m_walletman.Add(wallet);
-    }
+    void AddWallet(const std::shared_ptr<CWallet>& wallet) override { m_walletman.Add(wallet); }
     void RemoveWallet(const std::string& name) override
     {
         m_walletman.Remove(name);

src/coinjoin/interfaces.cpp

@@ -87,14 +87,15 @@ void CKeyHolderStorage::ReturnAll()
     }
 }
 
-CTransactionBuilderOutput::CTransactionBuilderOutput(CTransactionBuilder* pTxBuilderIn, std::shared_ptr<CWallet> pwalletIn, CAmount nAmountIn) :
+CTransactionBuilderOutput::CTransactionBuilderOutput(CTransactionBuilder* pTxBuilderIn,
+                                                     const std::shared_ptr<CWallet>& wallet, CAmount nAmountIn) :
     pTxBuilder(pTxBuilderIn),
-    dest(pwalletIn.get()),
+    dest(wallet.get()),
     nAmount(nAmountIn)
 {
     assert(pTxBuilder);
     CTxDestination txdest;
-    LOCK(pwalletIn->cs_wallet);
+    LOCK(wallet->cs_wallet);
     dest.GetReservedDestination(txdest, false);
     script = ::GetScriptForDestination(txdest);
 }
@@ -108,15 +109,15 @@ bool CTransactionBuilderOutput::UpdateAmount(const CAmount nNewAmount)
     return true;
 }
 
-CTransactionBuilder::CTransactionBuilder(std::shared_ptr<CWallet> pwalletIn, const CompactTallyItem& tallyItemIn) :
-    pwallet(pwalletIn),
-    dummyReserveDestination(pwalletIn.get()),
+CTransactionBuilder::CTransactionBuilder(const std::shared_ptr<CWallet>& wallet, const CompactTallyItem& tallyItemIn) :
+    m_wallet(wallet),
+    dummyReserveDestination(wallet.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);
+    coinControl.m_discard_feerate = ::GetDiscardRate(*m_wallet);
     // Generate a feerate which will be used by calculations of this class and also by CWallet::CreateTransaction
-    coinControl.m_feerate = std::max(GetRequiredFeeRate(*pwallet), pwallet->m_pay_tx_fee);
+    coinControl.m_feerate = std::max(GetRequiredFeeRate(*m_wallet), m_wallet->m_pay_tx_fee);
     // Change always goes back to origin
     coinControl.destChange = tallyItemIn.txdest;
     // Only allow tallyItems inputs for tx creation
@@ -131,16 +132,16 @@ CTransactionBuilder::CTransactionBuilder(std::shared_ptr<CWallet> pwalletIn, con
     // Get a comparable dummy scriptPubKey, avoid writing/flushing to the actual wallet db
     CScript dummyScript;
     {
-        LOCK(pwallet->cs_wallet);
-        WalletBatch dummyBatch(pwallet->GetDatabase(), false);
+        LOCK(m_wallet->cs_wallet);
+        WalletBatch dummyBatch(m_wallet->GetDatabase(), false);
         dummyBatch.TxnBegin();
         CKey secret;
-        secret.MakeNewKey(pwallet->CanSupportFeature(FEATURE_COMPRPUBKEY));
+        secret.MakeNewKey(m_wallet->CanSupportFeature(FEATURE_COMPRPUBKEY));
         CPubKey dummyPubkey = secret.GetPubKey();
         dummyBatch.TxnAbort();
         dummyScript = ::GetScriptForDestination(PKHash(dummyPubkey));
         // Calculate required bytes for the dummy signed tx with tallyItem's inputs only
-        nBytesBase = CalculateMaximumSignedTxSize(CTransaction(dummyTx), pwallet.get(), false);
+        nBytesBase = CalculateMaximumSignedTxSize(CTransaction(dummyTx), m_wallet.get(), false);
     }
     // Calculate the output size
     nBytesOutput = ::GetSerializeSize(CTxOut(0, dummyScript), PROTOCOL_VERSION);
@@ -204,7 +205,7 @@ CTransactionBuilderOutput* CTransactionBuilder::AddOutput(CAmount nAmountOutput)
 {
     if (CouldAddOutput(nAmountOutput)) {
         LOCK(cs_outputs);
-        vecOutputs.push_back(std::make_unique<CTransactionBuilderOutput>(this, pwallet, nAmountOutput));
+        vecOutputs.push_back(std::make_unique<CTransactionBuilderOutput>(this, m_wallet, nAmountOutput));
         return vecOutputs.back().get();
     }
     return nullptr;
@@ -233,12 +234,12 @@ CAmount CTransactionBuilder::GetAmountUsed() const
 CAmount CTransactionBuilder::GetFee(unsigned int nBytes) const
 {
     CAmount nFeeCalc = coinControl.m_feerate->GetFee(nBytes);
-    CAmount nRequiredFee = GetRequiredFee(*pwallet, nBytes);
+    CAmount nRequiredFee = GetRequiredFee(*m_wallet, nBytes);
     if (nRequiredFee > nFeeCalc) {
         nFeeCalc = nRequiredFee;
     }
-    if (nFeeCalc > pwallet->m_default_max_tx_fee) {
-        nFeeCalc = pwallet->m_default_max_tx_fee;
+    if (nFeeCalc > m_wallet->m_default_max_tx_fee) {
+        nFeeCalc = m_wallet->m_default_max_tx_fee;
     }
     return nFeeCalc;
 }
@@ -273,9 +274,9 @@ bool CTransactionBuilder::Commit(bilingual_str& strResult)
 
     CTransactionRef tx;
     {
-        LOCK2(pwallet->cs_wallet, cs_main);
+        LOCK2(m_wallet->cs_wallet, cs_main);
         FeeCalculation fee_calc_out;
-        if (!pwallet->CreateTransaction(vecSend, tx, nFeeRet, nChangePosRet, strResult, coinControl, fee_calc_out)) {
+        if (!m_wallet->CreateTransaction(vecSend, tx, nFeeRet, nChangePosRet, strResult, coinControl, fee_calc_out)) {
             return false;
         }
     }
@@ -312,8 +313,8 @@ bool CTransactionBuilder::Commit(bilingual_str& strResult)
     }
 
     {
-        LOCK2(pwallet->cs_wallet, cs_main);
-        pwallet->CommitTransaction(tx, {}, {});
+        LOCK2(m_wallet->cs_wallet, cs_main);
+        m_wallet->CommitTransaction(tx, {}, {});
     }
 
     fKeepKeys = true;

src/coinjoin/util.cpp

@@ -54,7 +54,7 @@ class CTransactionBuilderOutput
     CScript script;
 
 public:
-    CTransactionBuilderOutput(CTransactionBuilder* pTxBuilderIn, std::shared_ptr<CWallet> pwalletIn, CAmount nAmountIn);
+    CTransactionBuilderOutput(CTransactionBuilder* pTxBuilderIn, const std::shared_ptr<CWallet>& wallet, CAmount nAmountIn);
     CTransactionBuilderOutput(CTransactionBuilderOutput&&) = delete;
     CTransactionBuilderOutput& operator=(CTransactionBuilderOutput&&) = delete;
     /// Get the scriptPubKey of this output
@@ -77,7 +77,7 @@ class CTransactionBuilderOutput
 class CTransactionBuilder
 {
     /// Wallet the transaction will be build for
-    std::shared_ptr<CWallet> pwallet;
+    const std::shared_ptr<CWallet>& m_wallet;
     /// See CTransactionBuilder() for initialization
     CCoinControl coinControl;
     /// Dummy since we anyway use tallyItem's destination as change destination in coincontrol.
@@ -100,7 +100,7 @@ class CTransactionBuilder
     friend class CTransactionBuilderOutput;
 
 public:
-    CTransactionBuilder(std::shared_ptr<CWallet> pwalletIn, const CompactTallyItem& tallyItemIn);
+    CTransactionBuilder(const std::shared_ptr<CWallet>& wallet, const CompactTallyItem& tallyItemIn);
     ~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 EXCLUSIVE_LOCKS_REQUIRED(!cs_outputs);

src/coinjoin/util.h

@@ -24,6 +24,7 @@
 
 static const std::string MNEHF_REQUESTID_PREFIX = "mnhf";
 static const std::string DB_SIGNALS = "mnhf_s";
+static const std::string DB_SIGNALS_v2 = "mnhf_s2";
 
 uint256 MNHFTxPayload::GetRequestId() const
 {
@@ -57,34 +58,33 @@ CMNHFManager::Signals CMNHFManager::GetSignalsStage(const CBlockIndex* const pin
 {
     if (!DeploymentActiveAfter(pindexPrev, Params().GetConsensus(), Consensus::DEPLOYMENT_V20)) return {};
 
-    Signals signals = GetForBlock(pindexPrev);
+    Signals signals_tmp = GetForBlock(pindexPrev);
+
     if (pindexPrev == nullptr) return {};
     const int height = pindexPrev->nHeight + 1;
-    for (auto it = signals.begin(); it != signals.end(); ) {
-        bool found{false};
-        const auto signal_pindex = pindexPrev->GetAncestor(it->second);
+
+    Signals signals_ret;
+
+    for (auto signal : signals_tmp) {
+        bool expired{false};
+        const auto signal_pindex = pindexPrev->GetAncestor(signal.second);
         assert(signal_pindex != nullptr);
         const int64_t signal_time = signal_pindex->GetMedianTimePast();
         for (int index = 0; index < Consensus::MAX_VERSION_BITS_DEPLOYMENTS; ++index) {
             const auto& deployment = Params().GetConsensus().vDeployments[index];
-            if (deployment.bit != it->first) continue;
+            if (deployment.bit != signal.first) continue;
             if (signal_time < deployment.nStartTime) {
                 // new deployment is using the same bit as the old one
-                LogPrintf("CMNHFManager::GetSignalsStage: mnhf signal bit=%d height:%d is expired at height=%d\n", it->first, it->second, height);
-                it = signals.erase(it);
-            } else {
-                ++it;
+                LogPrintf("CMNHFManager::GetSignalsStage: mnhf signal bit=%d height:%d is expired at height=%d\n",
+                          signal.first, signal.second, height);
+                expired = true;
             }
-            found = true;
-            break;
         }
-        if (!found) {
-            // no deployment means we buried it and aren't using the same bit (yet)
-            LogPrintf("CMNHFManager::GetSignalsStage: mnhf signal bit=%d height:%d is not known at height=%d\n", it->first, it->second, height);
-            it = signals.erase(it);
+        if (!expired) {
+            signals_ret.insert(signal);
         }
     }
-    return signals;
+    return signals_ret;
 }
 
 bool MNHFTx::Verify(const llmq::CQuorumManager& qman, const uint256& quorumHash, const uint256& requestId, const uint256& msgHash, TxValidationState& state) const
@@ -287,6 +287,9 @@ CMNHFManager::Signals CMNHFManager::GetForBlock(const CBlockIndex* pindex)
     const Consensus::Params& consensusParams{Params().GetConsensus()};
     while (!to_calculate.empty()) {
         const CBlockIndex* pindex_top{to_calculate.top()};
+        if (pindex_top->nHeight % 1000 == 0) {
+            LogPrintf("re-index EHF signals at block %d\n", pindex_top->nHeight);
+        }
         CBlock block;
         if (!ReadBlockFromDisk(block, pindex_top, consensusParams)) {
             throw std::runtime_error("failed-getehfforblock-read");
@@ -328,11 +331,19 @@ std::optional<CMNHFManager::Signals> CMNHFManager::GetFromCache(const CBlockInde
             return signals;
         }
     }
-    if (m_evoDb.Read(std::make_pair(DB_SIGNALS, blockHash), signals)) {
+    if (m_evoDb.Read(std::make_pair(DB_SIGNALS_v2, blockHash), signals)) {
         LOCK(cs_cache);
         mnhfCache.insert(blockHash, signals);
         return signals;
     }
+    if (!DeploymentActiveAt(*pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_MN_RR)) {
+        // before mn_rr activation we are safe
+        if (m_evoDb.Read(std::make_pair(DB_SIGNALS, blockHash), signals)) {
+            LOCK(cs_cache);
+            mnhfCache.insert(blockHash, signals);
+            return signals;
+        }
+    }
     return std::nullopt;
 }
 
@@ -346,7 +357,7 @@ void CMNHFManager::AddToCache(const Signals& signals, const CBlockIndex* const p
     }
     if (!DeploymentActiveAt(*pindex, Params().GetConsensus(), Consensus::DEPLOYMENT_V20)) return;
 
-    m_evoDb.Write(std::make_pair(DB_SIGNALS, blockHash), signals);
+    m_evoDb.Write(std::make_pair(DB_SIGNALS_v2, blockHash), signals);
 }
 
 void CMNHFManager::AddSignal(const CBlockIndex* const pindex, int bit)
@@ -364,6 +375,25 @@ void CMNHFManager::ConnectManagers(gsl::not_null<ChainstateManager*> chainman, g
     m_qman = qman;
 }
 
+bool CMNHFManager::ForceSignalDBUpdate()
+{
+    // force ehf signals db update
+    auto dbTx = m_evoDb.BeginTransaction();
+
+    const bool last_legacy = bls::bls_legacy_scheme.load();
+    bls::bls_legacy_scheme.store(false);
+    GetSignalsStage(m_chainman->ActiveChainstate().m_chain.Tip());
+    bls::bls_legacy_scheme.store(last_legacy);
+
+    dbTx->Commit();
+    // flush it to disk
+    if (!m_evoDb.CommitRootTransaction()) {
+        LogPrintf("CMNHFManager::%s -- failed to commit to evoDB\n", __func__);
+        return false;
+    }
+    return true;
+}
+
 std::string MNHFTx::ToString() const
 {
     return strprintf("MNHFTx(versionBit=%d, quorumHash=%s, sig=%s)",

src/evo/mnhftx.cpp

@@ -155,6 +155,8 @@ class CMNHFManager : public AbstractEHFManager
      */
     void DisconnectManagers() { m_chainman = nullptr; m_qman = nullptr; };
 
+    bool ForceSignalDBUpdate() EXCLUSIVE_LOCKS_REQUIRED(cs_main);
+
 private:
     void AddToCache(const Signals& signals, const CBlockIndex* const pindex);
 

src/evo/mnhftx.h

@@ -2064,6 +2064,10 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
                     strLoadError = _("Error upgrading evo database");
                     break;
                 }
+                if (!node.mnhf_manager->ForceSignalDBUpdate()) {
+                    strLoadError = _("Error upgrading evo database for EHF");
+                    break;
+                }
 
                 for (CChainState* chainstate : chainman.GetAll()) {
                     if (!is_coinsview_empty(chainstate)) {

src/init.cpp

@@ -33,7 +33,7 @@ class Loader
 public:
     virtual ~Loader() {}
     //! Add new wallet to CoinJoin client manager
-    virtual void AddWallet(CWallet&) = 0;
+    virtual void AddWallet(const std::shared_ptr<CWallet>&) = 0;
     //! Remove wallet from CoinJoin client manager
     virtual void RemoveWallet(const std::string&) = 0;
     virtual void FlushWallet(const std::string&) = 0;

src/interfaces/coinjoin.h

@@ -23,7 +23,7 @@ class TransactionFilterProxy : public QSortFilterProxyModel
     /** Type filter bit field (all types) */
     static const quint32 ALL_TYPES = 0xFFFFFFFF;
     /** Type filter bit field (all types but Darksend-SPAM) */
-    static const quint32 COMMON_TYPES = 4223;
+    static const quint32 COMMON_TYPES = 0x307f;
 
     static quint32 TYPE(int type) { return 1<<type; }