From 4652cd0d828a14c64b896d1c4d435231bc4d0c50 Mon Sep 17 00:00:00 2001
From: furszy <matiasfurszyfer@protonmail.com>
Date: Wed, 25 Mar 2026 10:09:31 -0400
Subject: [PATCH] txdb: detect UTXO deserialization errors via
 CDBWrapper::TryRead()

If a UTXO entry on disk can't be deserialized, the node treats it
as if the coin doesn't exist. Any block that spends that coin is
permanently rejected as invalid (BLOCK_FAILED_VALID), silently
forking the node from the rest of the network. This can hardly be
triggered in practice (details below), but it's still the wrong
behavior that could affect us in the future.

The root cause is that CDBWrapper::Read() returns false for both
missing keys and deserialization failures, so the consensus
class CCoinsViewDB::GetCoin() has no way to tell them apart.
CCoinsViewErrorCatcher was built to catch database read errors
and abort, but it never fires because CDBWrapper::Read() swallows
the exception before it can propagate.

In practice, this scenario isn't a latent risk at the moment. It
requires either a bug in the coin serialization path, or memory
corruption before the data reaches LevelDB (at which point we have
bigger problems). Any random disk-level bit flips are caught earlier
by LevelDB's verification (the verify_checksums=true option enabled
by default), which surfaces as a DatabaseError rather than a
deserialization failure.

This commit switches CCoinsViewDB::GetCoin() to use
CDBWrapper::TryRead(), which lets the caller discriminate between
all possible outcomes. On deserialization error, the exception now
propagates through CCoinsViewErrorCatcher to ExecuteBackedWrapper(),
which invokes the shutdown callbacks and aborts the node accordantly.

This also fixes PeekCoin(), which delegates to GetCoin() at the
CCoinsViewDB level.
---
 src/txdb.cpp | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/src/txdb.cpp b/src/txdb.cpp
index 2c39cf2767b..d5c00620d14 100644
--- a/src/txdb.cpp
+++ b/src/txdb.cpp
@@ -71,11 +71,24 @@ void CCoinsViewDB::ResizeCache(size_t new_cache_size)
 
 std::optional<Coin> CCoinsViewDB::GetCoin(const COutPoint& outpoint) const
 {
-    if (Coin coin; m_db->Read(CoinEntry(&outpoint), coin)) {
-        Assert(!coin.IsSpent()); // The UTXO database should never contain spent coins
-        return coin;
+    Coin coin;
+    const CDBWrapper::ReadStatus res = m_db->TryRead(CoinEntry(&outpoint), coin);
+    if (!res) {
+        // Propagate errors so CCoinsViewErrorCatcher triggers a clean shutdown.
+        switch (const auto& [err_code, err_msg] = res.error(); err_code) {
+            case CDBWrapper::ReadFailure::Code::DeserializationError:
+                throw dbwrapper_error{strprintf("Coin deserialization failure: %s", err_msg)};
+            case CDBWrapper::ReadFailure::Code::DatabaseError:
+                throw dbwrapper_error{strprintf("Coin DB read failure: %s", err_msg)};
+        } // no default case, so the compiler can warn about missing cases
+        std::abort(); // unreachable
     }
-    return std::nullopt;
+
+    // Check whether the coin exists
+    if (!res.value()) return std::nullopt;
+    // Coin found, ensure UTXO database never contains spent coins
+    Assert(!coin.IsSpent());
+    return coin;
 }
 
 bool CCoinsViewDB::HaveCoin(const COutPoint &outpoint) const {