doc: improve CoinsViewOverlay documentation

Document the single main thread requirement, clarify how
FetchCoinFromBase consumes fetched inputs, and note why Reset must stop
fetching. Also explain a no-op StartFetching call in the unit tests.

Co-authored-by: Ryan Ofsky <ryan@ofsky.org>

src/coins.h

@@ -556,12 +556,16 @@ private:
 };
 
 /**
- * CCoinsViewCache subclass that asynchronously fetches most block input prevouts in parallel during ConnectBlock without
- * mutating the base cache.
+ * CCoinsViewCache subclass that asynchronously fetches most block input prevouts in parallel during ConnectBlock
+ * without mutating the base cache. This is achieved by fetching coins from the base view using PeekCoin() instead of
+ * GetCoin(), so intermediate CCoinsViewCache layers are not filled.
  *
- * Only used in ConnectBlock to pass as an ephemeral view that can be reset if the block is invalid.
- * It provides the same interface as CCoinsViewCache.
- * It adds an additional StartFetching method to provide the block.
+ * Used during ConnectBlock() as an ephemeral, resettable top-level view that is flushed only on success, so invalid
+ * blocks don't pollute the underlying cache.
+ *
+ * While this class uses threads internally to fetch coins, externally it is only safe to call its methods from a
+ * single "main" thread. It assumes StartFetching, StopFetching, FetchCoinFromBase, Flush and Reset will all only be
+ * called from the main thread.
  *
  * When a block is passed to StartFetching, the inputs of the block are flattened into a vector of InputToFetch
  * objects. StartFetching then submits worker tasks to a ThreadPool and keeps the returned futures alive until fetching
@@ -571,17 +575,16 @@ private:
  * m_inputs vector at a time. Workers race to claim inputs, so they may fetch elements in any order. If the fetched
  * index is greater than or equal to the size of m_inputs, no more inputs can be fetched and false is returned.
  *
- * The worker claims the InputToFetch at this index, fetches the coin from the base cache and moves it into the
+ * The worker claims the InputToFetch at this index, fetches the coin with base->PeekCoin() and moves it into the
  * InputToFetch object. The ready flag is then set with a release memory order. This allows the ready flag to be
  * used as a memory fence, guaranteeing the coin being written to the object will have happened before another
  * thread tests the flag with an acquire memory order.
- * This assumes all base->PeekCoin() paths are safe for concurrent readers and do not mutate lower cache layers.
+ * This assumes all base->PeekCoin() paths are safe for concurrent readers.
  *
- * When a coin is requested from the cache on the main thread and is not already in cacheCoins map, FetchCoinFromBase
- * checks whether the next unconsumed entry in m_inputs has the requested outpoint. On a match, m_input_tail is advanced
- * and the entry's ready flag is waited on with an acquire memory order until a worker has finished fetching it. The
- * coin is then moved out and returned. Since the main thread is the only consumer of validation results, it blocks
- * on the specific input it needs rather than racing workers for other inputs.
+ * The main thread is the only consumer of the fetched coins. FetchCoinFromBase is called when a coin is requested on
+ * the main thread and is not already in the cache. It checks whether the next unconsumed entry in m_inputs has the
+ * requested outpoint. On a match, m_input_tail is advanced and the entry's ready flag is waited on with an acquire
+ * memory order until a worker has finished fetching it. The coin is then moved out and returned.
  *
  * StopFetching() is called in Flush() and in Reset() (the per-block teardown) so workers stop before the block they
  * reference goes away. It stops fetching by moving m_input_head to the end of m_inputs (so workers quickly exit),
@@ -705,6 +708,9 @@ private:
     std::vector<std::future<void>> m_futures{};
 
 protected:
+    //! StopFetching must be called here for two reasons: InputToFetch objects hold references to the
+    //! block's outpoints, so they must not outlive the block being connected; and when connecting a
+    //! block fails, workers must not keep fetching inputs for the block that was abandoned.
     void Reset() noexcept override
     {
         StopFetching();

src/test/coinsviewoverlay_tests.cpp

@@ -203,6 +203,8 @@ BOOST_AUTO_TEST_CASE(access_non_input_coins)
     main_cache.EmplaceCoinInternalDANGER(COutPoint{outpoint}, std::move(coin));
 
     CoinsViewOverlay view{&main_cache, MakeStartedThreadPool()};
+    // The block has no non-coinbase transactions, so this fetches nothing and only creates the
+    // reset guard. All lookups below use the fallback path.
     const auto reset_guard{view.StartFetching(block)};
 
     // Non-input fallback hit.