[wallet] Remove locked_chain from CWallet, its RPCs and tests

This change is intended to make the bitcoin node and its rpc, network
and gui interfaces more responsive while the wallet is in use. Currently
because the node's cs_main mutex is always locked before the wallet's
cs_wallet mutex (to prevent deadlocks), cs_main currently stays locked
while the wallet does relatively slow things like creating and listing
transactions.

This commit only remmove chain lock tacking in wallet code, and invert
lock order from cs_main, cs_wallet to cs_wallet, cs_main.
must happen at once to avoid any deadlock. Previous commit were only
removing Chain::Lock methods to Chain interface and enforcing they
take cs_main.

Remove LockChain method from CWallet and Chain::Lock interface.

1 files changed, 9 insertions, 21 deletions

diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h
index d35d386925..e33fe54ac8 100644
--- a/src/interfaces/chain.h
+++ b/src/interfaces/chain.h
@@ -59,12 +59,7 @@ public:
 //! internal workings of the bitcoin node, and not being very convenient to use.
 //! Chain methods should be cleaned up and simplified over time. Examples:
 //!
-//! * The Chain::lock() method, which lets clients delay chain tip updates
-//!   should be removed when clients are able to respond to updates
-//!   asynchronously
-//!   (https://github.com/bitcoin/bitcoin/pull/10973#issuecomment-380101269).
-//!
-//! * The initMessage() and showProgress() methods which the wallet uses to send
+//! * The initMessages() and showProgress() methods which the wallet uses to send
 //!   notifications to the GUI should go away when GUI and wallet can directly
 //!   communicate with each other without going through the node
 //!   (https://github.com/bitcoin/bitcoin/pull/15288#discussion_r253321096).
@@ -72,26 +67,19 @@ public:
 //! * The handleRpc, registerRpcs, rpcEnableDeprecated methods and other RPC
 //!   methods can go away if wallets listen for HTTP requests on their own
 //!   ports instead of registering to handle requests on the node HTTP port.
+//!
+//! * Move fee estimation queries to an asynchronous interface and let the
+//!   wallet cache it, fee estimation being driven by node mempool, wallet
+//!   should be the consumer.
+//!
+//! * The `guessVerificationProgress`, `getBlockHeight`, `getBlockHash`, etc
+//!   methods can go away if rescan logic is moved on the node side, and wallet
+//!   only register rescan request.
 class Chain
 {
 public:
     virtual ~Chain() {}
 
-    //! Interface for querying locked chain state, used by legacy code that
-    //! assumes state won't change between calls. New code should avoid using
-    //! the Lock interface and instead call higher-level Chain methods
-    //! that return more information so the chain doesn't need to stay locked
-    //! between calls.
-    class Lock
-    {
-    public:
-        virtual ~Lock() {}
-    };
-
-    //! Return Lock interface. Chain is locked when this is called, and
-    //! unlocked when the returned interface is freed.
-    virtual std::unique_ptr<Lock> lock(bool try_lock = false) = 0;
-
     //! Get current chain height, not including genesis block (returns 0 if
     //! chain only contains genesis block, nullopt if chain does not contain
     //! any blocks)