From c1694ce6bb7e19a8722d5583cd85ad17da40bb67 Mon Sep 17 00:00:00 2001
From: Russell Yanofsky <russ@yanofsky.org>
Date: Thu, 16 Jan 2020 16:38:30 -0500
Subject: [PATCH] wallet: Avoid use of Chain::Lock in importprunedfunds

This is a step toward removing the Chain::Lock class and reducing cs_main
locking.

This change only affects behavior in the case where wallet last block processed
falls behind the chain tip, in which case the "Block not found in chain" error
will be stricter and not allow importing data from a blocks between the wallet
last processed tip and the current node tip.
---
 src/interfaces/chain.cpp      |  8 ++++++++
 src/interfaces/chain.h        |  6 ++++++
 src/test/interfaces_tests.cpp | 10 ++++++++++
 src/wallet/rpcdump.cpp        | 11 ++++++-----
 4 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp
index cfaf79f709e66..1c1fbe7387d86 100644
--- a/src/interfaces/chain.cpp
+++ b/src/interfaces/chain.cpp
@@ -267,6 +267,14 @@ class ChainImpl : public Chain
         WAIT_LOCK(cs_main, lock);
         return FillBlock(LookupBlockIndex(hash), block, lock);
     }
+    bool findAncestorByHash(const uint256& block_hash, const uint256& ancestor_hash, const FoundBlock& ancestor_out) override
+    {
+        WAIT_LOCK(cs_main, lock);
+        const CBlockIndex* block = LookupBlockIndex(block_hash);
+        const CBlockIndex* ancestor = LookupBlockIndex(ancestor_hash);
+        if (block && ancestor && block->GetAncestor(ancestor->nHeight) != ancestor) ancestor = nullptr;
+        return FillBlock(ancestor, ancestor_out, lock);
+    }
     void findCoins(std::map<COutPoint, Coin>& coins) override { return FindCoins(m_node, coins); }
     double guessVerificationProgress(const uint256& block_hash) override
     {
diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h
index 3778ab9a8bb2b..7504f4cfb6008 100644
--- a/src/interfaces/chain.h
+++ b/src/interfaces/chain.h
@@ -150,6 +150,12 @@ class Chain
     //! or contents.
     virtual bool findBlock(const uint256& hash, const FoundBlock& block={}) = 0;
 
+    //! Return whether block descends from a specified ancestor, and
+    //! optionally return ancestor information.
+    virtual bool findAncestorByHash(const uint256& block_hash,
+        const uint256& ancestor_hash,
+        const FoundBlock& ancestor_out={}) = 0;
+
     //! Look up unspent output information. Returns coins in the mempool and in
     //! the current chain UTXO set. Iterates through all the keys in the map and
     //! populates the values.
diff --git a/src/test/interfaces_tests.cpp b/src/test/interfaces_tests.cpp
index e3d1738c7ff38..caa988df0d184 100644
--- a/src/test/interfaces_tests.cpp
+++ b/src/test/interfaces_tests.cpp
@@ -44,4 +44,14 @@ BOOST_AUTO_TEST_CASE(findBlock)
     BOOST_CHECK(!chain->findBlock({}, FoundBlock()));
 }
 
+BOOST_AUTO_TEST_CASE(findAncestorByHash)
+{
+    auto chain = interfaces::MakeChain(m_node);
+    auto& active = ChainActive();
+    int height = -1;
+    BOOST_CHECK(chain->findAncestorByHash(active[20]->GetBlockHash(), active[10]->GetBlockHash(), FoundBlock().height(height)));
+    BOOST_CHECK_EQUAL(height, 10);
+    BOOST_CHECK(!chain->findAncestorByHash(active[10]->GetBlockHash(), active[20]->GetBlockHash()));
+}
+
 BOOST_AUTO_TEST_SUITE_END()
diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp
index e4d0a3fa6d5fd..1d6b4832eb469 100644
--- a/src/wallet/rpcdump.cpp
+++ b/src/wallet/rpcdump.cpp
@@ -28,6 +28,8 @@
 
 
 
+using interfaces::FoundBlock;
+
 std::string static EncodeDumpString(const std::string &str) {
     std::stringstream ret;
     for (const unsigned char c : str) {
@@ -359,8 +361,9 @@ UniValue importprunedfunds(const JSONRPCRequest& request)
     }
 
     auto locked_chain = pwallet->chain().lock();
-    Optional<int> height = locked_chain->getBlockHeight(merkleBlock.header.GetHash());
-    if (height == nullopt) {
+    LOCK(pwallet->cs_wallet);
+    int height;
+    if (!pwallet->chain().findAncestorByHash(pwallet->GetLastBlockHash(), merkleBlock.header.GetHash(), FoundBlock().height(height))) {
         throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found in chain");
     }
 
@@ -371,11 +374,9 @@ UniValue importprunedfunds(const JSONRPCRequest& request)
 
     unsigned int txnIndex = vIndex[it - vMatch.begin()];
 
-    CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, *height, merkleBlock.header.GetHash(), txnIndex);
+    CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, height, merkleBlock.header.GetHash(), txnIndex);
     wtx.m_confirm = confirm;
 
-    LOCK(pwallet->cs_wallet);
-
     if (pwallet->IsMine(*wtx.tx)) {
         pwallet->AddToWallet(wtx, false);
         return NullUniValue;