From f0e498af5c0b9978bcd97b26556c83f54beb63f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Tue, 21 Apr 2026 11:56:41 +0200 Subject: [PATCH 1/2] test: cover failed `CDBIterator::GetKey()` deserialization The upcoming change will replace the temporary owning `DataStream` inside `CDBIterator::GetKey()` with a borrowed reader over the current LevelDB key bytes. The copied `DataStream` currently insulates the iterator entry from a failed decode, so the optimization is only safe if a deserialization failure still returns `false` and leaves the same key/value readable afterward. Extend `dbwrapper_iterator` to read a one-byte key as a `uint16_t`. The read must fail, return `false`, and still allow the same key and value to be read afterward. This would fail if `GetKey()` stopped swallowing deserialization exceptions, or if a failed decode started consuming shared iterator state instead of only temporary reader state. Drop the dead `const_cast` in the test while here, since `dbw` is already non-const. Locking down that contract first makes the following `SpanReader` switch a behavior-preserving optimization. --- src/test/dbwrapper_tests.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/test/dbwrapper_tests.cpp b/src/test/dbwrapper_tests.cpp index 3896ea64da5..39fa75a3af4 100644 --- a/src/test/dbwrapper_tests.cpp +++ b/src/test/dbwrapper_tests.cpp @@ -201,11 +201,15 @@ BOOST_AUTO_TEST_CASE(dbwrapper_iterator) uint256 in2 = m_rng.rand256(); dbw.Write(key2, in2); - std::unique_ptr it(const_cast(dbw).NewIterator()); + std::unique_ptr it(dbw.NewIterator()); // Be sure to seek past the obfuscation key (if it exists) it->Seek(key); + // A failed key decode must not consume the current iterator entry. + uint16_t key_too_large{0}; + BOOST_CHECK(!it->GetKey(key_too_large)); + uint8_t key_res; uint256 val_res; From 5de2f97a0521fe75b47f9840141c4ace74656de9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Tue, 21 Apr 2026 11:56:49 +0200 Subject: [PATCH 2/2] dbwrapper: use `SpanReader` for iterator keys `CDBIterator::GetKey()` only deserializes the current LevelDB key once. `GetKeyImpl()` already exposes the current key as a contiguous borrowed byte span, and `GetKey()` creates a fresh local reader and only performs immediate forward reads before returning. Switch this path to `SpanReader` so the key bytes are read in place instead of being copied into a temporary `DataStream`. This keeps the same exception swallowing and `bool` return semantics while avoiding the extra allocation and copy. The preceding test locks down the subtle safety property that matters here: a failed decode must not consume the current iterator entry. Note that the same simplification does not apply to `GetValue()`, because that path deobfuscates the value bytes in place first and still needs an owning mutable buffer. --- src/dbwrapper.cpp | 2 ++ src/dbwrapper.h | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/dbwrapper.cpp b/src/dbwrapper.cpp index eb222078b5e..50a58d5fe03 100644 --- a/src/dbwrapper.cpp +++ b/src/dbwrapper.cpp @@ -370,6 +370,8 @@ void CDBIterator::SeekImpl(std::span key) std::span CDBIterator::GetKeyImpl() const { + // The returned span borrows from the current iterator entry and is only + // valid until the iterator is advanced. return MakeByteSpan(m_impl_iter->iter->key()); } diff --git a/src/dbwrapper.h b/src/dbwrapper.h index 2eee6c1c023..75ac5fdb05b 100644 --- a/src/dbwrapper.h +++ b/src/dbwrapper.h @@ -153,7 +153,7 @@ public: template bool GetKey(K& key) { try { - DataStream ssKey{GetKeyImpl()}; + SpanReader ssKey{GetKeyImpl()}; ssKey >> key; } catch (const std::exception&) { return false;