Merge bitcoin/bitcoin#24103: Replace RecursiveMutex `m_cs_chainstate` with Mutex, and rename it

020acea99b605c9b5ee7939a6acef131db84ad4a refactor: replace RecursiveMutex m_chainstate_mutex with Mutex (w0xlt)
ddeefeef20fa2fe48c3c4563370a6297704d228e refactor: add negative TS annotations for `m_chainstate_mutex` (w0xlt)
1dfd31bc267c54144a7e62ad5a1a5860c032f4d7 scripted-diff: rename m_cs_chainstate -> m_chainstate_mutex (w0xlt)

Pull request description:

  This PR is related to #19303 and gets rid of the `RecursiveMutex m_cs_chainstate`.

  `m_cs_chainstate` is only held in `ActivateBestChain()` and `InvalidateBlock()`.
  So apparently there is no recursion involved, so the `m_cs_chainstate` can be a non-recursive mutex.

ACKs for top commit:
  hebasto:
    ACK 020acea99b605c9b5ee7939a6acef131db84ad4a, I have reviewed the code and it looks OK, I agree it can be merged.
  theStack:
    Code-review ACK 020acea99b605c9b5ee7939a6acef131db84ad4a 🌴
  shaavan:
    reACK 020acea99b605c9b5ee7939a6acef131db84ad4a

Tree-SHA512: c7c16e727e326df3410514915ce753a2a5e1da78857ef965ef683e36251e1b73c9cced4cd5231b04dbe2be0ea14084f6731b4d7a4d9a8e086e982b985e37e4b4

1 files changed, 7 insertions, 3 deletions

diff --git a/src/validation.cpp b/src/validation.cpp
index 5fca1551c8..c12dc9e8b6 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -2847,6 +2847,8 @@ static void LimitValidationInterfaceQueue() LOCKS_EXCLUDED(cs_main) {
 
 bool CChainState::ActivateBestChain(BlockValidationState& state, std::shared_ptr<const CBlock> pblock)
 {
+    AssertLockNotHeld(m_chainstate_mutex);
+
     // Note that while we're often called here from ProcessNewBlock, this is
     // far from a guarantee. Things in the P2P/RPC will often end up calling
     // us in the middle of ProcessNewBlock - do not assume pblock is set
@@ -2856,8 +2858,8 @@ bool CChainState::ActivateBestChain(BlockValidationState& state, std::shared_ptr
     // ABC maintains a fair degree of expensive-to-calculate internal state
     // because this function periodically releases cs_main so that it does not lock up other threads for too long
     // during large connects - and to allow for e.g. the callback queue to drain
-    // we use m_cs_chainstate to enforce mutual exclusion so that only one caller may execute this function at a time
-    LOCK(m_cs_chainstate);
+    // we use m_chainstate_mutex to enforce mutual exclusion so that only one caller may execute this function at a time
+    LOCK(m_chainstate_mutex);
 
     CBlockIndex *pindexMostWork = nullptr;
     CBlockIndex *pindexNewTip = nullptr;
@@ -2976,6 +2978,8 @@ bool CChainState::PreciousBlock(BlockValidationState& state, CBlockIndex* pindex
 
 bool CChainState::InvalidateBlock(BlockValidationState& state, CBlockIndex* pindex)
 {
+    AssertLockNotHeld(m_chainstate_mutex);
+
     // Genesis block can't be invalidated
     assert(pindex);
     if (pindex->nHeight == 0) return false;
@@ -2987,7 +2991,7 @@ bool CChainState::InvalidateBlock(BlockValidationState& state, CBlockIndex* pind
     // We do not allow ActivateBestChain() to run while InvalidateBlock() is
     // running, as that could cause the tip to change while we disconnect
     // blocks.
-    LOCK(m_cs_chainstate);
+    LOCK(m_chainstate_mutex);
 
     // We'll be acquiring and releasing cs_main below, to allow the validation
     // callbacks to run. However, we should keep the block index in a