refactor: inline CCoinsViewErrorCatcher into CCoinsViewDB #34132

1. 
   l0rinc commented at 10:56 pm on December 20, 2025: contributor

   Problem

   The current coins database read-error handling adds unnecessary complexity to the critical path:

   • CCoinsViewErrorCatcher sits between the cache and database views, wrapping every read operation with a generic error handler
   • ExecuteBackedWrapper adds another function call layer with lambda captures on each database read
   • Uses std::vector<std::function<void()>> for callbacks when there’s only ever one callback registered
   • The error callback is set after database initialization rather than during construction (requiring a cs_main lock)

   This adds unnecessary stack depth and bookkeeping overhead on every coins database read operation (GetCoin). Also note that CCoinsViewDB::HaveCoin isn’t even called from production code as far as I can tell.

   Context

   This is part of ongoing coins caching cleanup efforts like #34124, #33866, #33018, and #33512.

   Fix

   This PR collapses the CCoinsViewErrorCatcher layer entirely and moves error handling directly into CCoinsViewDB:

   • Remove ExecuteBackedWrapper and CCoinsViewErrorCatcher, placing try-catch blocks directly in CCoinsViewDB::GetCoin and CCoinsViewDB::HaveCoin
   • Replace std::vector<std::function<void()>> with a single std::function<void()> member, initialized in the constructor
   • The read error callback is now passed to Chainstate::InitCoinsDB and forwarded to CCoinsViewDB constructor

   A new test is also added to reproduce the behavior in case of database corruption. It’s added as a first commit to demonstrate that the behavior stays unchanged.

2. DrahtBot added the label Refactoring on Dec 20, 2025
3. 
   DrahtBot commented at 10:56 pm on December 20, 2025: contributor

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Code Coverage & Benchmarks

   For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/34132.

   Reviews

   See the guideline for information on the review process. A summary of reviews will appear here.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #34164 (validation: add reusable coins view for ConnectBlock by andrewtoth)
   • #31382 (kernel: Flush in ChainstateManager destructor by sedited)
   • #31132 (validation: fetch block inputs on parallel threads 3x faster IBD by andrewtoth)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

4. DrahtBot added the label CI failed on Dec 21, 2025
5. l0rinc force-pushed on Dec 21, 2025
6. test: add test for coins database read error handling

   The error was also reworded to avoid the trailing repetition (see expected stderr).
   
   The test failure can be triggered manually by:
   ```patch
    std::optional<Coin> CCoinsViewErrorCatcher::GetCoin(const COutPoint& outpoint) const
    {
   -    return ExecuteBackedWrapper<std::optional<Coin>>([&]() { return CCoinsViewBacked::GetCoin(outpoint); }, m_err_callbacks);
   +    return CCoinsViewBacked::GetCoin(outpoint);
    }
   ```
   
   Note that `CCoinsViewErrorCatcher::HaveCoin` failure isn't tested since it doesn't seem to be used in production code, all tests pass with:
   ```patch
    bool CCoinsViewErrorCatcher::HaveCoin(const COutPoint& outpoint) const
    {
   -    return ExecuteBackedWrapper<bool>([&]() { return CCoinsViewBacked::HaveCoin(outpoint); }, m_err_callbacks);
   +    throw "CCoinsViewBacked::HaveCoin";
    }
   ```
   
   `skip_if_no_ipc` was added to the test since on `i686, no IPC` the db corruption didn't trigger any failure.

   83d2b3eaca
7. refactor: rename coins_error_cb to read_error_cb for clarity 76020f7f32
8. refactor: inline `ExecuteBackedWrapper`

   Inlined `ExecuteBackedWrapper`, removing a lambda indirection and specializing the error logs per usage.
   
   The error handling will be pushed down to the call sites in follow ups.

   96d132cdc9
9. refactor: inline `CCoinsViewErrorCatcher`

   There's only ever a single callback at most, let's assign it instead of pretending we have a list of callbacks. This will be moved to the constructor in the next commit.

   e81c38e3c7
10. refactor: init read error callback in InitCoinsDB 0973a03b76
11. l0rinc force-pushed on Dec 22, 2025
12. 
    l0rinc commented at 8:04 am on December 23, 2025: contributor

    close/open dance to retrigger CI - likely caused by GitHub outage
13. l0rinc closed this on Dec 23, 2025

    ---

14. l0rinc reopened this on Dec 23, 2025

    ---

15. DrahtBot removed the label CI failed on Dec 23, 2025
16. l0rinc marked this as ready for review on Dec 23, 2025

Contributors
l0rinc DrahtBot

Labels
Refactoring