[backport#15288] Remove uses of CheckFinalTx in wallet code

Summary:
80f52a2267f44a9cae4440615df3ff989be1579c This commit does not change behavior. (Russell Yanofsky)

---

This is a follow up to D5804 and a partial backport of Core [[bitcoin/bitcoin#15288 | PR15288]]

Test Plan:
  cmake .. -DENABLE_WERROR=ON -DCMAKE_BUILD_TYPE=Debug -DBUILD_BITCOIN_WALLET=OFF
  ninja check check-functional
  cmake .. -DENABLE_WERROR=ON -DCMAKE_BUILD_TYPE=Debug
  ninja check check-functional

Reviewers: #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Subscribers: deadalnix

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

src/interfaces/chain.cpp

@@ -9,6 +9,7 @@
 #include <primitives/block.h>
 #include <primitives/blockhash.h>
 #include <sync.h>
+#include <threadsafety.h>
 #include <util/system.h>
 #include <validation.h>
 
@@ -133,6 +134,12 @@ namespace {
             }
             return nullopt;
         }
+        bool contextualCheckTransactionForCurrentBlock(
+            const Consensus::Params &params, const CTransaction &tx,
+            CValidationState &state) override {
+            LockAnnotation lock(::cs_main);
+            return ContextualCheckTransactionForCurrentBlock(params, tx, state);
+        }
     };
 
     class LockingStateImpl : public LockImpl,

src/interfaces/chain.h

@@ -17,6 +17,11 @@ class CBlock;
 struct CBlockLocator;
 class CChainParams;
 class CScheduler;
+class CTransaction;
+class CValidationState;
+namespace Consensus {
+struct Params;
+}
 
 namespace interfaces {
 
@@ -106,6 +111,11 @@ class Chain {
         //! is guaranteed to be an ancestor of the block used to create the
         //! locator.
         virtual Optional<int> findLocatorFork(const CBlockLocator &locator) = 0;
+
+        //! Check if transaction will be final given chain height current time.
+        virtual bool contextualCheckTransactionForCurrentBlock(
+            const Consensus::Params &params, const CTransaction &tx,
+            CValidationState &state) = 0;
     };
 
     //! Return Lock interface. Chain is locked when this is called, and

src/interfaces/wallet.cpp

@@ -98,8 +98,7 @@ namespace {
     static WalletTxStatus
     MakeWalletTxStatus(interfaces::Chain::Lock &locked_chain,
                        const CWalletTx &wtx) {
-        // Temporary, for ContextualCheckTransactionForCurrentBlock below.
-        // Removed in upcoming commit.
+        // Temporary, for LookupBlockIndex below. Removed in upcoming commit.
         LockAnnotation lock(::cs_main);
 
         WalletTxStatus result;
@@ -111,8 +110,9 @@ namespace {
         result.time_received = wtx.nTimeReceived;
         result.lock_time = wtx.tx->nLockTime;
         CValidationState state;
-        result.is_final = ContextualCheckTransactionForCurrentBlock(
-            Params().GetConsensus(), *wtx.tx, state);
+        result.is_final =
+            locked_chain.contextualCheckTransactionForCurrentBlock(
+                Params().GetConsensus(), *wtx.tx, state);
         result.is_trusted = wtx.IsTrusted(locked_chain);
         result.is_abandoned = wtx.isAbandoned();
         result.is_coinbase = wtx.IsCoinBase();

src/wallet/rpcwallet.cpp

@@ -711,9 +711,6 @@ static UniValue getreceivedbyaddress(const Config &config,
     // the user could have gotten from another RPC command prior to now
     pwallet->BlockUntilSyncedToCurrentChain();
 
-    // Temporary, for ContextualCheckTransactionForCurrentBlock below. Removed
-    // in upcoming commit.
-    LockAnnotation lock(::cs_main);
     auto locked_chain = pwallet->chain().lock();
     LOCK(pwallet->cs_wallet);
 
@@ -742,7 +739,7 @@ static UniValue getreceivedbyaddress(const Config &config,
 
         CValidationState state;
         if (wtx.IsCoinBase() ||
-            !ContextualCheckTransactionForCurrentBlock(
+            !locked_chain->contextualCheckTransactionForCurrentBlock(
                 config.GetChainParams().GetConsensus(), *wtx.tx, state)) {
             continue;
         }
@@ -807,9 +804,6 @@ static UniValue getreceivedbylabel(const Config &config,
     // the user could have gotten from another RPC command prior to now
     pwallet->BlockUntilSyncedToCurrentChain();
 
-    // Temporary, for ContextualCheckTransactionForCurrentBlock below. Removed
-    // in upcoming commit.
-    LockAnnotation lock(::cs_main);
     auto locked_chain = pwallet->chain().lock();
     LOCK(pwallet->cs_wallet);
 
@@ -829,7 +823,7 @@ static UniValue getreceivedbylabel(const Config &config,
         const CWalletTx &wtx = pairWtx.second;
         CValidationState state;
         if (wtx.IsCoinBase() ||
-            !ContextualCheckTransactionForCurrentBlock(
+            !locked_chain->contextualCheckTransactionForCurrentBlock(
                 config.GetChainParams().GetConsensus(), *wtx.tx, state)) {
             continue;
         }
@@ -1264,10 +1258,6 @@ static UniValue
 ListReceived(const Config &config, interfaces::Chain::Lock &locked_chain,
              CWallet *const pwallet, const UniValue &params, bool by_label)
     EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) {
-    // Temporary, for ContextualCheckTransactionForCurrentBlock below. Removed
-    // in upcoming commit.
-    LockAnnotation lock(::cs_main);
-
     // Minimum confirmations
     int nMinDepth = 1;
     if (!params[0].isNull()) {
@@ -1305,7 +1295,7 @@ ListReceived(const Config &config, interfaces::Chain::Lock &locked_chain,
 
         CValidationState state;
         if (wtx.IsCoinBase() ||
-            !ContextualCheckTransactionForCurrentBlock(
+            !locked_chain.contextualCheckTransactionForCurrentBlock(
                 config.GetChainParams().GetConsensus(), *wtx.tx, state)) {
             continue;
         }

src/wallet/wallet.cpp

@@ -2183,14 +2183,10 @@ bool CWalletTx::InMempool() const {
 }
 
 bool CWalletTx::IsTrusted(interfaces::Chain::Lock &locked_chain) const {
-    // Temporary, for ContextualCheckTransactionForCurrentBlock below. Removed
-    // in upcoming commit.
-    LockAnnotation lock(::cs_main);
-
     // Quick answer in most cases
     CValidationState state;
-    if (!ContextualCheckTransactionForCurrentBlock(Params().GetConsensus(), *tx,
-                                                   state)) {
+    if (!locked_chain.contextualCheckTransactionForCurrentBlock(
+            Params().GetConsensus(), *tx, state)) {
         return false;
     }
 
@@ -2400,9 +2396,6 @@ Amount CWallet::GetImmatureWatchOnlyBalance() const {
 // trusted.
 Amount CWallet::GetLegacyBalance(const isminefilter &filter,
                                  int minDepth) const {
-    // Temporary, for ContextualCheckTransactionForCurrentBlock below. Removed
-    // in upcoming commit.
-    LockAnnotation lock(::cs_main);
     auto locked_chain = chain().lock();
     LOCK(cs_wallet);
 
@@ -2414,8 +2407,8 @@ Amount CWallet::GetLegacyBalance(const isminefilter &filter,
         const int depth = wtx.GetDepthInMainChain(*locked_chain);
         CValidationState state;
         if (depth < 0 ||
-            !ContextualCheckTransactionForCurrentBlock(params, *wtx.tx,
-                                                       state) ||
+            !locked_chain->contextualCheckTransactionForCurrentBlock(
+                params, *wtx.tx, state) ||
             wtx.IsImmatureCoinBase(*locked_chain)) {
             continue;
         }
@@ -2477,8 +2470,8 @@ void CWallet::AvailableCoins(interfaces::Chain::Lock &locked_chain,
         const CWalletTx *pcoin = &entry.second;
 
         CValidationState state;
-        if (!ContextualCheckTransactionForCurrentBlock(params, *pcoin->tx,
-                                                       state)) {
+        if (!locked_chain.contextualCheckTransactionForCurrentBlock(
+                params, *pcoin->tx, state)) {
             continue;
         }