doc: Add comment to cs_main and mempool::cs

src/txmempool.h

@@ -485,7 +485,43 @@ class CTxMemPool
  >
  > indexed_transaction_set;
 
- mutable CCriticalSection cs;
+ /**
+ * This mutex needs to be locked when accessing `mapTx` or other members
+ * that are guarded by it.
+ *
+ * @par Consistency guarantees
+ *
+ * By design, it is guaranteed that:
+ *
+ * 1. Locking both `cs_main` and `mempool.cs` will give a view of mempool
+ * that is consistent with current chain tip (`chainActive` and
+ * `pcoinsTip`) and is fully populated. Fully populated means that if the
+ * current active chain is missing transactions that were present in a
+ * previously active chain, all the missing transactions will have been
+ * re-added to the mempool and should be present if they meet size and
+ * consistency constraints.
+ *
+ * 2. Locking `mempool.cs` without `cs_main` will give a view of a mempool
+ * consistent with some chain that was active since `cs_main` was last
+ * locked, and that is fully populated as described above. It is ok for
+ * code that only needs to query or remove transactions from the mempool
+ * to lock just `mempool.cs` without `cs_main`.
+ *
+ * To provide these guarantees, it is necessary to lock both `cs_main` and
+ * `mempool.cs` whenever adding transactions to the mempool and whenever
+ * changing the chain tip. It's necessary to keep both mutexes locked until
+ * the mempool is consistent with the new chain tip and fully populated.
+ *
+ * @par Consistency bug
+ *
+ * The second guarantee above is not currently enforced, but
+ * https://github.com/bitcoin/bitcoin/pull/14193 will fix it. No known code
+ * in bitcoin currently depends on second guarantee, but it is important to
+ * fix for third party code that needs be able to frequently poll the
+ * mempool without locking `cs_main` and without encountering missing
+ * transactions during reorgs.
+ */
+ mutable RecursiveMutex cs;
  indexed_transaction_set mapTx GUARDED_BY(cs);
 
  using txiter = indexed_transaction_set::nth_index<0>::type::const_iterator;

src/validation.cpp

@@ -210,9 +210,17 @@ class CChainState {
  bool RollforwardBlock(const CBlockIndex* pindex, CCoinsViewCache& inputs, const CChainParams& params) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
 } g_chainstate;
 
-
-
-CCriticalSection cs_main;
+/**
+ * Mutex to guard access to validation specific variables, such as reading
+ * or changing the chainstate.
+ *
+ * This may also need to be locked when updating the transaction pool, e.g. on
+ * AcceptToMemoryPool. See CTxMemPool::cs comment for details.
+ *
+ * The transaction pool has a separate lock to allow reading from it and the
+ * chainstate at the same time.
+ */
+RecursiveMutex cs_main;
 
 BlockMap& mapBlockIndex = g_chainstate.mapBlockIndex;
 CChain& chainActive = g_chainstate.chainActive;