test: cover failed `CDBIterator::GetKey()` deserialization
What the developer wrote
Authored by Lőrinc
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.
What changed, and why it matters
This commit only adds a new test to Bitcoin Core. It checks that when a database iterator tries to read a small one-byte key as a larger type and fails, the iterator is not broken and the same key/value can still be read afterward. The change itself is not a security fix; it is preparation for a future optimization in how keys are read from LevelDB.
No action required. This is a test-only commit that strengthens an existing contract. Review the follow-up optimization commit when it appears to confirm it preserves the tested behavior.
Security signals we found
Adds regression test for deserialization failure handling in database iterator
Commit message references future optimization and behavior-preserving contract
No production code changes; test-only commit
Evidence from the diff
The diff extends dbwrapper_tests.cpp with a regression test for CDBIterator::GetKey(). It writes a one-byte key, attempts to deserialize it into a uint16_t (which must fail), and then verifies the iterator remains usable for reading the original key and value. The commit message explains this locks down a contract before replacing a temporary owning DataStream with a borrowed SpanReader. A dead const_cast is also removed.
Changed components
src/test/dbwrapper_tests.cppInspect captured patch +5 / −1
diff --git a/src/test/dbwrapper_tests.cpp b/src/test/dbwrapper_tests.cpp
index 3896ea64..39fa75a3 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<CDBIterator> it(const_cast<CDBWrapper&>(dbw).NewIterator());
+ std::unique_ptr<CDBIterator> 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;
Why this scored 19/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.