Merge #16033: Hold cs_main when reading chainActive via getTipLocator(). Remove assumeLocked().

9402ef0739 Remove temporary method assumeLocked(). Remove LockingStateImpl. Remove redundant cs_main locks. (practicalswift)
593a8e8a2c wallet: Use chain.lock() instead of temporary chain.assumeLocked() (practicalswift)

Pull request description:

  Fixes #16028.

  Problem description:

  `LockAnnotation lock(::cs_main)` is a guarantee to the compiler thread analysis that `::cs_main` is locked (when it couldn't be determined otherwise).

  Despite being annotated with the locking guarantee ...

  https://github.com/bitcoin/bitcoin/blob/65526fc8666fef35ef908dbc225f706bef642c7e/src/interfaces/chain.cpp#L134-L138

  ... `getTipLocator()` reads `chainActive` (via `::ChainActive()`) without holding `cs_main`.

  This can be verified by adding the following `AssertLockHeld(cs_main)`:

  ```
  $ git diff
  diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp
  index 59623284d..9fc693a0f 100644
  --- a/src/interfaces/chain.cpp
  +++ b/src/interfaces/chain.cpp
  @@ -134,6 +134,7 @@ class LockImpl : public Chain::Lock
       CBlockLocator getTipLocator() override
       {
           LockAnnotation lock(::cs_main);
  +        AssertLockHeld(::cs_main);
           return ::ChainActive().GetLocator();
       }
       Optional<int> findLocatorFork(const CBlockLocator& locator) override
  $ make check
  ../build-aux/test-driver: line 107: 12881 Aborted                 "$@" > $log_file 2>&1
  FAIL: qt/test/test_bitcoin-qt
  ```

ACKs for commit 9402ef:
  MarcoFalke:
    utACK 9402ef0739fdcd8e989c07c0595095e9608b243c
  ryanofsky:
    utACK 9402ef0739fdcd8e989c07c0595095e9608b243c. Changes are consolidating commits and removing redundant lock2 cs_main calls

Tree-SHA512: 0a030bf0c07eb53194ecc246f973ef389dd42a0979f51932bf94bdf7e90c52473ae03be49718ee1629582b05dd8e0dc020b5a210318c93378ea4ace90c0f9f72

1 files changed, 0 insertions, 5 deletions

diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h
index 1d604bd823..e675defd47 100644
--- a/src/interfaces/chain.h
+++ b/src/interfaces/chain.h
@@ -138,11 +138,6 @@ public:
     //! unlocked when the returned interface is freed.
     virtual std::unique_ptr<Lock> lock(bool try_lock = false) = 0;
 
-    //! Return Lock interface assuming chain is already locked. This
-    //! method is temporary and is only used in a few places to avoid changing
-    //! behavior while code is transitioned to use the Chain::Lock interface.
-    virtual std::unique_ptr<Lock> assumeLocked() = 0;
-
     //! Return whether node has the block and optionally return block metadata
     //! or contents.
     //!