From 5dfbb91b6cc5d1e0e3e49dad1fe9dddc4b06dfba Mon Sep 17 00:00:00 2001 From: furszy Date: Tue, 24 Mar 2026 23:51:41 -0400 Subject: [PATCH] dbwrapper: add TryRead() to distinguish errors from valid outcomes Read() returns false for both a missing key and a deserialization failure, making it impossible for callers to distinguish between them. This commits adds TryRead() returning a ReadStatus struct that discriminates between: - true: record found, value deserialized - false: record not found - DatabaseError: levelDB threw during record read - DeserializationError: key present, value incompatible with expected format An err_msg field preserves the original exception message for diagnostic purposes. This also makes Read() a thin wrapper over TryRead() to keep existing call sites unchanged. Note: Key serialization is the only operation that may throw in TryRead(), as callers are expected to provide well-formed keys. This is why this function is not noexcept. --- src/dbwrapper.h | 71 +++++++++++++++++++++++++++++++++--- src/test/dbwrapper_tests.cpp | 50 +++++++++++++++++++++++++ 2 files changed, 115 insertions(+), 6 deletions(-) diff --git a/src/dbwrapper.h b/src/dbwrapper.h index 2eee6c1c023..b6696ea4450 100644 --- a/src/dbwrapper.h +++ b/src/dbwrapper.h @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -203,26 +204,84 @@ public: CDBWrapper(const CDBWrapper&) = delete; CDBWrapper& operator=(const CDBWrapper&) = delete; + struct ReadFailure { + enum class Code { + DeserializationError, //!< Key exists but value could not be deserialized. + DatabaseError, //!< Unexpected internal DB error. + }; + + Code status; + std::string err_msg; + }; + + using ReadStatus = util::Expected; + + /** + * Read and deserialize a value from the database, with explicit error discrimination. + * + * Unlike Read(), this method distinguishes between a missing key, a deserialization + * failure (DeserializationError), and an internal DB error (DatabaseError), + * enabling callers to treat data corruption differently from an absent entry. + * + * @note Callers are expected to provide well-formed keys; key serialization + * is the only operation that may throw. + * + * @param[in] key The key to look up. + * @param[out] value Populated with the deserialized value when the returned + * Expected holds true; indeterminate otherwise. + * @return On success, true if the key was found (value populated) or false if + * the key was absent. On failure, a ReadFailure describing the error. + */ template - bool Read(const K& key, V& value) const + [[nodiscard]] ReadStatus TryRead(const K& key, V& value) const { DataStream ssKey{}; ssKey.reserve(DBWRAPPER_PREALLOC_KEY_SIZE); + // Key serialization is the only operation that may throw. + // Callers are expected to provide well-formed keys. ssKey << key; - std::optional strValue{ReadImpl(ssKey)}; - if (!strValue) { - return false; + + std::optional strValue; + try { + strValue = ReadImpl(ssKey); + if (!strValue) { + return false; // not found + } + } catch (const std::exception& e) { + return util::Unexpected(ReadFailure{ReadFailure::Code::DatabaseError, e.what()}); } + try { std::span ssValue{MakeWritableByteSpan(*strValue)}; m_obfuscation(ssValue); SpanReader{ssValue} >> value; - } catch (const std::exception&) { - return false; + } catch (const std::exception& e) { + return util::Unexpected(ReadFailure{ReadFailure::Code::DeserializationError, e.what()}); } + return true; } + /** + * Wrapper around TryRead() that preserves the original Read() semantics: + * returns true on success, false if the key is absent or deserialization + * fails, and throws dbwrapper_error on an internal DB error. + * + * Prefer TryRead() when the caller needs to distinguish between a missing + * key and a corrupt value. + */ + template + bool Read(const K& key, V& value) const + { + const ReadStatus res = TryRead(key,value); + if (res.has_value()) return res.value(); + switch (const auto& [err_code, err_msg] = res.error(); err_code) { + case ReadFailure::Code::DeserializationError: return false; + case ReadFailure::Code::DatabaseError: throw dbwrapper_error(err_msg); + } // no default case, so the compiler can warn about missing cases + std::abort(); // unreachable + } + template void Write(const K& key, const V& value, bool fSync = false) { diff --git a/src/test/dbwrapper_tests.cpp b/src/test/dbwrapper_tests.cpp index 4eb31c27526..fcac00b7aef 100644 --- a/src/test/dbwrapper_tests.cpp +++ b/src/test/dbwrapper_tests.cpp @@ -239,6 +239,56 @@ BOOST_AUTO_TEST_CASE(dbwrapper_read_throws_on_db_error) const auto db{make_db(path, /*force_compact=*/false)}; uint256 result; BOOST_CHECK_EXCEPTION(db.Read(key, result), dbwrapper_error, HasReason("Fatal LevelDB error")); + + // TryRead() must return DatabaseError (without throwing). + CDBWrapper::ReadStatus status = db.TryRead(key, result); + BOOST_REQUIRE(!status); + BOOST_CHECK(status.error().status == CDBWrapper::ReadFailure::Code::DatabaseError); + BOOST_CHECK(status.error().err_msg.find("Fatal LevelDB error") != std::string::npos); +} + +// Exercise TryRead() return values directly: found, absent and DeserializationError. +// DatabaseError is tested inside 'dbwrapper_read_throws_on_db_error' test +BOOST_AUTO_TEST_CASE(dbwrapper_tryread) +{ + for (const bool obfuscate : {false, true}) { + const fs::path path{m_args.GetDataDirBase() / (obfuscate ? "dbwrapper_tryread_obf" : "dbwrapper_tryread_noobf")}; + CDBWrapper dbw({.path = path, .cache_bytes = 1 << 20, .wipe_data = true, .obfuscate = obfuscate}); + + constexpr uint8_t key_ok{'A'}; + constexpr uint8_t key_missing{'B'}; + constexpr uint8_t key_bad{'C'}; + + uint256 written_value{m_rng.rand256()}; + dbw.Write(key_ok, written_value); + dbw.Write(key_bad, uint8_t{0xFF}); + + // Found: key exists, value deserializes correctly + { + uint256 read_value; + CDBWrapper::ReadStatus status = dbw.TryRead(key_ok, read_value); + BOOST_REQUIRE(status); + BOOST_CHECK(status.value()); + BOOST_CHECK_EQUAL(read_value, written_value); + } + + // Absent: key does not exist + { + uint256 read_value; + CDBWrapper::ReadStatus status = dbw.TryRead(key_missing, read_value); + BOOST_REQUIRE(status); + BOOST_CHECK(!status.value()); + } + + // DeserializationError: key exists but stored value is too short + { + uint256 read_value; + CDBWrapper::ReadStatus status = dbw.TryRead(key_bad, read_value); + BOOST_REQUIRE(!status); + BOOST_CHECK(status.error().status == CDBWrapper::ReadFailure::Code::DeserializationError); + BOOST_CHECK(!status.error().err_msg.empty()); + } + } } BOOST_AUTO_TEST_CASE(dbwrapper_iterator)