[backport#10973] Remove remaining wallet accesses to node globals

Summary:
bitcoin/bitcoin@d358466 Remove remaining wallet accesses to node globals (Russell Yanofsky)

---

Depends on D5927, completes T601

This is a partial backport of Core [[bitcoin/bitcoin#10973 | PR10973]]

Test Plan:
  cmake .. -GNinja -DCMAKE_BUILD_TYPE=Debug
  ninja check check-functional

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Subscribers: Fabien

Differential Revision: https://reviews.bitcoinabc.org/D5943

src/interfaces/chain.cpp

@@ -11,12 +11,14 @@
 #include <net.h>
 #include <node/coin.h>
 #include <policy/mempool.h>
+#include <policy/policy.h>
 #include <primitives/block.h>
 #include <primitives/blockhash.h>
 #include <primitives/transaction.h>
 #include <protocol.h>
 #include <rpc/protocol.h>
 #include <rpc/server.h>
+#include <shutdown.h>
 #include <sync.h>
 #include <threadsafety.h>
 #include <timedata.h>
@@ -341,12 +343,15 @@ namespace {
                 limit_descendant_count, limit_descendant_size,
                 unused_error_string);
         }
+        CFeeRate relayMinFee() override { return ::minRelayTxFee; }
+        CFeeRate relayDustFee() override { return ::dustRelayFee; }
         Amount maxTxFee() override { return ::maxTxFee; }
         bool getPruneMode() override { return ::fPruneMode; }
         bool p2pEnabled() override { return g_connman != nullptr; }
         bool isInitialBlockDownload() override {
             return IsInitialBlockDownload();
         }
+        bool shutdownRequested() override { return ShutdownRequested(); }
         int64_t getAdjustedTime() override { return GetAdjustedTime(); }
         void initMessage(const std::string &message) override {
             ::uiInterface.InitMessage(message);
@@ -360,6 +365,10 @@ namespace {
         void loadWallet(std::unique_ptr<Wallet> wallet) override {
             ::uiInterface.LoadWallet(wallet);
         }
+        void showProgress(const std::string &title, int progress,
+                          bool resume_possible) override {
+            ::uiInterface.ShowProgress(title, progress, resume_possible);
+        }
         std::unique_ptr<Handler>
         handleNotifications(Notifications &notifications) override {
             return std::make_unique<NotificationsHandlerImpl>(*this,

src/interfaces/chain.h

@@ -200,6 +200,13 @@ class Chain {
     //! Check if transaction will pass the mempool's chain limits.
     virtual bool checkChainLimits(const CTransactionRef &tx) = 0;
 
+    //! Relay current minimum fee (from -minrelaytxfee settings).
+    virtual CFeeRate relayMinFee() = 0;
+
+    //! Relay dust fee setting (-dustrelayfee), reflecting lowest rate it's
+    //! economical to spend.
+    virtual CFeeRate relayDustFee() = 0;
+
     //! Get node max tx fee setting (-maxtxfee).
     //! This could be replaced by a per-wallet max fee, as proposed at
     //! https://github.com/bitcoin/bitcoin/issues/15355
@@ -212,9 +219,12 @@ class Chain {
     //! Check if p2p enabled.
     virtual bool p2pEnabled() = 0;
 
-    // Check if in IBD.
+    //! Check if in IBD.
     virtual bool isInitialBlockDownload() = 0;
 
+    //! Check if shutdown requested.
+    virtual bool shutdownRequested() = 0;
+
     //! Get adjusted time.
     virtual int64_t getAdjustedTime() = 0;
 
@@ -230,6 +240,10 @@ class Chain {
     //! Send wallet load notification to the GUI.
     virtual void loadWallet(std::unique_ptr<Wallet> wallet) = 0;
 
+    //! Send progress indicator.
+    virtual void showProgress(const std::string &title, int progress,
+                              bool resume_possible) = 0;
+
     //! Chain notifications.
     class Notifications {
     public:

src/interfaces/wallet.cpp

@@ -98,13 +98,10 @@ namespace {
     static WalletTxStatus
     MakeWalletTxStatus(interfaces::Chain::Lock &locked_chain,
                        const CWalletTx &wtx) {
-        // Temporary, for LookupBlockIndex below. Removed in upcoming commit.
-        LockAnnotation lock(::cs_main);
-
         WalletTxStatus result;
-        CBlockIndex *block = LookupBlockIndex(wtx.hashBlock);
         result.block_height =
-            (block ? block->nHeight : std::numeric_limits<int>::max());
+            locked_chain.getBlockHeight(wtx.hashBlock)
+                .get_value_or(std::numeric_limits<int>::max());
         result.blocks_to_maturity = wtx.GetBlocksToMaturity(locked_chain);
         result.depth_in_main_chain = wtx.GetDepthInMainChain(locked_chain);
         result.time_received = wtx.nTimeReceived;

src/wallet/fees.cpp

@@ -23,15 +23,16 @@ Amount GetMinimumFee(const CWallet &wallet, unsigned int nTxBytes,
         GetMinimumFeeRate(wallet, coin_control, pool).GetFeeCeiling(nTxBytes);
 
     // But always obey the maximum.
-    if (nFeeNeeded > maxTxFee) {
-        nFeeNeeded = maxTxFee;
+    const Amount max_tx_fee = wallet.chain().maxTxFee();
+    if (nFeeNeeded > max_tx_fee) {
+        nFeeNeeded = max_tx_fee;
     }
 
     return nFeeNeeded;
 }
 
 CFeeRate GetRequiredFeeRate(const CWallet &wallet) {
-    return std::max(wallet.m_min_fee, ::minRelayTxFee);
+    return std::max(wallet.m_min_fee, wallet.chain().relayMinFee());
 }
 
 CFeeRate GetMinimumFeeRate(const CWallet &wallet,

src/wallet/init.cpp

@@ -282,7 +282,7 @@ bool VerifyWallets(const CChainParams &chainParams, interfaces::Chain &chain,
 
     LogPrintf("Using wallet directory %s\n", GetWalletDir().string());
 
-    uiInterface.InitMessage(_("Verifying wallet(s)..."));
+    chain.initMessage(_("Verifying wallet(s)..."));
 
     // Parameter interaction code should have thrown an error if -salvagewallet
     // was enabled with more than wallet file, so the wallet_files size check

src/wallet/rpcdump.cpp

@@ -701,13 +701,13 @@ UniValue importwallet(const Config &config, const JSONRPCRequest &request) {
         // uiInterface.ShowProgress does not have a cancel button.
 
         // show progress dialog in GUI
-        uiInterface.ShowProgress(
+        pwallet->chain().showProgress(
             strprintf("%s " + _("Importing..."), pwallet->GetDisplayName()), 0,
             false);
         std::vector<std::tuple<CKey, int64_t, bool, std::string>> keys;
         std::vector<std::pair<CScript, int64_t>> scripts;
         while (file.good()) {
-            uiInterface.ShowProgress(
+            pwallet->chain().showProgress(
                 "",
                 std::max(1, std::min<int>(50, 100 * double(file.tellg()) /
                                                   double(nFilesize))),
@@ -758,15 +758,15 @@ UniValue importwallet(const Config &config, const JSONRPCRequest &request) {
         if (keys.size() > 0 &&
             pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) {
             // hide progress dialog in GUI
-            uiInterface.ShowProgress("", 100, false);
+            pwallet->chain().showProgress("", 100, false);
             throw JSONRPCError(
                 RPC_WALLET_ERROR,
                 "Importing wallets is disabled when private keys are disabled");
         }
         double total = double(keys.size() + scripts.size());
         double progress = 0;
         for (const auto &key_tuple : keys) {
-            uiInterface.ShowProgress(
+            pwallet->chain().showProgress(
                 "",
                 std::max(50, std::min<int>(75, 100 * progress / total) + 50),
                 false);
@@ -798,7 +798,7 @@ UniValue importwallet(const Config &config, const JSONRPCRequest &request) {
             progress++;
         }
         for (const auto &script_pair : scripts) {
-            uiInterface.ShowProgress(
+            pwallet->chain().showProgress(
                 "",
                 std::max(50, std::min<int>(75, 100 * progress / total) + 50),
                 false);
@@ -824,11 +824,11 @@ UniValue importwallet(const Config &config, const JSONRPCRequest &request) {
             progress++;
         }
         // hide progress dialog in GUI
-        uiInterface.ShowProgress("", 100, false);
+        pwallet->chain().showProgress("", 100, false);
         pwallet->UpdateTimeFirstKey(nTimeBegin);
     }
     // hide progress dialog in GUI
-    uiInterface.ShowProgress("", 100, false);
+    pwallet->chain().showProgress("", 100, false);
     RescanWallet(*pwallet, reserver, nTimeBegin, false /* update */);
     pwallet->MarkDirty();
 

src/wallet/rpcwallet.cpp

@@ -2904,11 +2904,11 @@ static UniValue settxfee(const Config &config, const JSONRPCRequest &request) {
     CFeeRate tx_fee_rate(nAmount, 1000);
     if (tx_fee_rate == CFeeRate()) {
         // automatic selection
-    } else if (tx_fee_rate < ::minRelayTxFee) {
+    } else if (tx_fee_rate < pwallet->chain().relayMinFee()) {
         throw JSONRPCError(
             RPC_INVALID_PARAMETER,
             strprintf("txfee cannot be less than min relay tx fee (%s)",
-                      ::minRelayTxFee.ToString()));
+                      pwallet->chain().relayMinFee().ToString()));
     } else if (tx_fee_rate < pwallet->m_min_fee) {
         throw JSONRPCError(
             RPC_INVALID_PARAMETER,

src/wallet/wallet.cpp

@@ -1869,7 +1869,7 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(
                 stop_block.IsNull() ? tip_hash : stop_block);
         }
         double progress_current = progress_begin;
-        while (block_height && !fAbortRescan && !ShutdownRequested()) {
+        while (block_height && !fAbortRescan && !chain().shutdownRequested()) {
             if (*block_height % 100 == 0 &&
                 progress_end - progress_begin > 0.0) {
                 ShowProgress(
@@ -1950,7 +1950,7 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(
             WalletLogPrintf("Rescan aborted at block %d. Progress=%f\n",
                             block_height.value_or(0), progress_current);
             result.status = ScanResult::USER_ABORT;
-        } else if (block_height && ShutdownRequested()) {
+        } else if (block_height && chain().shutdownRequested()) {
             WalletLogPrintf("Rescan interrupted by shutdown request at block "
                             "%d. Progress=%f\n",
                             block_height.value_or(0), progress_current);
@@ -2453,7 +2453,6 @@ void CWallet::AvailableCoins(interfaces::Chain::Lock &locked_chain,
                              const Amount nMinimumSumAmount,
                              const uint64_t nMaximumCount, const int nMinDepth,
                              const int nMaxDepth) const {
-    AssertLockHeld(cs_main);
     AssertLockHeld(cs_wallet);
 
     vCoins.clear();
@@ -2574,7 +2573,6 @@ void CWallet::AvailableCoins(interfaces::Chain::Lock &locked_chain,
 
 std::map<CTxDestination, std::vector<COutput>>
 CWallet::ListCoins(interfaces::Chain::Lock &locked_chain) const {
-    AssertLockHeld(cs_main);
     AssertLockHeld(cs_wallet);
 
     std::map<CTxDestination, std::vector<COutput>> result;
@@ -2939,14 +2937,15 @@ bool CWallet::FundTransaction(CMutableTransaction &tx, Amount &nFeeRet,
     return true;
 }
 
-static bool IsCurrentForAntiFeeSniping(interfaces::Chain::Lock &locked_chain) {
-    if (IsInitialBlockDownload()) {
+static bool IsCurrentForAntiFeeSniping(interfaces::Chain &chain,
+                                       interfaces::Chain::Lock &locked_chain) {
+    if (chain.isInitialBlockDownload()) {
         return false;
     }
 
     // in seconds
     constexpr int64_t MAX_ANTI_FEE_SNIPING_TIP_AGE = 8 * 60 * 60;
-    if (ChainActive().Tip()->GetBlockTime() <
+    if (locked_chain.getBlockTime(*locked_chain.getHeight()) <
         (GetTime() - MAX_ANTI_FEE_SNIPING_TIP_AGE)) {
         return false;
     }
@@ -2958,7 +2957,8 @@ static bool IsCurrentForAntiFeeSniping(interfaces::Chain::Lock &locked_chain) {
  * current chain tip unless we are not synced with the current chain
  */
 static uint32_t
-GetLocktimeForNewTransaction(interfaces::Chain::Lock &locked_chain) {
+GetLocktimeForNewTransaction(interfaces::Chain &chain,
+                             interfaces::Chain::Lock &locked_chain) {
     uint32_t const height = locked_chain.getHeight().value_or(-1);
     uint32_t locktime;
     // Discourage fee sniping.
@@ -2981,7 +2981,7 @@ GetLocktimeForNewTransaction(interfaces::Chain::Lock &locked_chain) {
     // enough, that fee sniping isn't a problem yet, but by implementing a fix
     // now we ensure code won't be written that makes assumptions about
     // nLockTime that preclude a fix later.
-    if (IsCurrentForAntiFeeSniping(locked_chain)) {
+    if (IsCurrentForAntiFeeSniping(chain, locked_chain)) {
         locktime = height;
 
         // Secondly occasionally randomly pick a nLockTime even further back, so
@@ -3049,7 +3049,7 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock &locked_chainIn,
 
     CMutableTransaction txNew;
 
-    txNew.nLockTime = GetLocktimeForNewTransaction(locked_chainIn);
+    txNew.nLockTime = GetLocktimeForNewTransaction(chain(), locked_chainIn);
 
     {
         std::set<CInputCoin> setCoins;
@@ -3159,7 +3159,7 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock &locked_chainIn,
                 coin_selection_params.tx_noinputs_size +=
                     ::GetSerializeSize(txout, PROTOCOL_VERSION);
 
-                if (IsDust(txout, dustRelayFee)) {
+                if (IsDust(txout, chain().relayDustFee())) {
                     if (recipient.fSubtractFeeFromAmount &&
                         nFeeRet > Amount::zero()) {
                         if (txout.nValue < Amount::zero()) {
@@ -3253,7 +3253,7 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock &locked_chainIn,
             // If we made it here and we aren't even able to meet the relay fee
             // on the next pass, give up because we must be at the maximum
             // allowed fee.
-            if (nFeeNeeded < ::minRelayTxFee.GetFee(nBytes)) {
+            if (nFeeNeeded < chain().relayMinFee().GetFee(nBytes)) {
                 strFailReason = _("Transaction too large for fee policy");
                 return false;
             }
@@ -4655,11 +4655,11 @@ std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(
                   "send a transaction."));
         }
         walletInstance->m_pay_tx_fee = CFeeRate(nFeePerK, 1000);
-        if (walletInstance->m_pay_tx_fee < ::minRelayTxFee) {
+        if (walletInstance->m_pay_tx_fee < chain.relayMinFee()) {
             chain.initError(strprintf(
                 _("Invalid amount for -paytxfee=<amount>: '%s' "
                   "(must be at least %s)"),
-                gArgs.GetArg("-paytxfee", ""), ::minRelayTxFee.ToString()));
+                gArgs.GetArg("-paytxfee", ""), chain.relayMinFee().ToString()));
             return nullptr;
         }
     }
@@ -4842,8 +4842,6 @@ int CMerkleTx::GetDepthInMainChain(
         return 0;
     }
 
-    AssertLockHeld(cs_main);
-
     return locked_chain.getBlockDepth(hashBlock) * (nIndex == -1 ? -1 : 1);
 }
 

src/wallet/wallet.h

@@ -514,7 +514,7 @@ class CWalletTx : public CMerkleTx {
     Amount GetImmatureCredit(interfaces::Chain::Lock &locked_chain,
                              bool fUseCache = true) const;
     // TODO: Remove "NO_THREAD_SAFETY_ANALYSIS" and replace it with the correct
-    // annotation "EXCLUSIVE_LOCKS_REQUIRED(cs_main, pwallet->cs_wallet)". The
+    // annotation "EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet)". The
     // annotation "NO_THREAD_SAFETY_ANALYSIS" was temporarily added to avoid
     // having to resolve the issue of member access into incomplete type
     // CWallet.